[jbosscache-commits] JBoss Cache SVN: r4465 - in core/branches/1.4.X: src/org/jboss/cache/interceptors and 1 other directories.

Author: manik.surtani(a)jboss.com Date: 2007-09-17 09:14:29 -0400 (Mon, 17 Sep 2007) New Revision: 4465 Added: core/branches/1.4.X/src/org/jboss/cache/NodeCreationResult.java Modified: core/branches/1.4.X/src/org/jboss/cache/DataNode.java core/branches/1.4.X/src/org/jboss/cache/Node.java core/branches/1.4.X/src/org/jboss/cache/TransactionTable.java core/branches/1.4.X/src/org/jboss/cache/interceptors/PessimisticLockInterceptor.java core/branches/1.4.X/src/org/jboss/cache/interceptors/TxInterceptor.java core/branches/1.4.X/tests/functional/org/jboss/cache/transaction/AsyncRollbackTransactionManager.java core/branches/1.4.X/tests/functional/org/jboss/cache/transaction/SimultaneousRollbackAndPutTest.java Log: Fixes for JBCACHE-1165, JBCACHE-1166, JBCACHE-1168, JBCACHE-1164, JBCACHE-1183 Modified: core/branches/1.4.X/src/org/jboss/cache/DataNode.java =================================================================== --- core/branches/1.4.X/src/org/jboss/cache/DataNode.java 2007-09-14 17:18:10 UTC (rev 4464) +++ core/branches/1.4.X/src/org/jboss/cache/DataNode.java 2007-09-17 13:14:29 UTC (rev 4465) @@ -51,4 +51,14 @@ void unmarkForRemoval(boolean deep); void markForRemoval(); + /** + * This method hs identical functionality as getOrCreateChild, but returns a NodeCreationResult. + * If the node was created, NodeCreationResult#isCreated() returns true. + * @param name name of child to create + * @param tx transaction under which to create child + * @param createIfNotExists if false don't attempt to create the node if it doesn't exist + * @return a NodeCreationResult instance + */ + NodeCreationResult getOrCreateChildWithCreateInformation(Object name, GlobalTransaction tx, boolean createIfNotExists); + } Modified: core/branches/1.4.X/src/org/jboss/cache/Node.java =================================================================== --- core/branches/1.4.X/src/org/jboss/cache/Node.java 2007-09-14 17:18:10 UTC (rev 4464) +++ core/branches/1.4.X/src/org/jboss/cache/Node.java 2007-09-17 13:14:29 UTC (rev 4465) @@ -318,55 +318,64 @@ return this.data().put(key, value); } } + + public NodeCreationResult getOrCreateChildWithCreateInformation(Object child_name, GlobalTransaction gtx, boolean createIfNotExists){ + NodeCreationResult result = new NodeCreationResult(); + DataNode child; + if (child_name == null) + throw new IllegalArgumentException("null child name"); + child = (DataNode) children().get(child_name); + if (createIfNotExists && child == null) + { + if (log.isTraceEnabled()) log.trace("Children:"+children); + // construct the new child outside the synchronized block to avoid + // spending any more time than necessary in the synchronized section + Fqn child_fqn = new Fqn(this.fqn, child_name); + DataNode newChild = (DataNode) NodeFactory.getInstance().createNodeOfType(this, child_name, child_fqn, this, null, cache); + if (newChild == null) + throw new IllegalStateException(); + synchronized (this) + { + // check again to see if the child exists + // after acquiring exclusive lock + child = (Node) children().get(child_name); + if (child == null) + { + child = newChild; + children.put(child_name, child); + if (gtx != null) + { + MethodCall undo_op = MethodCallFactory.create(MethodDeclarations.removeNodeMethodLocal, + new Object[]{gtx, child_fqn, Boolean.FALSE}); + cache.addUndoOperation(gtx, undo_op); + // add the node name to the list maintained for the current tx + // (needed for abort/rollback of transaction) + // cache.addNode(gtx, child.getFqn()); + } + } + } + + // notify if we actually created a new child + if (newChild == child) + { + if (trace) + { + log.trace("created child: fqn=" + child_fqn); + } + result.setCreated(true); + cache.notifyNodeCreated(child.getFqn()); + }else{ + result.setCreated(false); + } + } + result.setTreeNode(child); + return result; + } + public TreeNode getOrCreateChild(Object child_name, GlobalTransaction gtx, boolean createIfNotExists) { - DataNode child; - if (child_name == null) - throw new IllegalArgumentException("null child name"); - - child = (DataNode) children().get(child_name); - if (createIfNotExists && child == null) - { - // construct the new child outside the synchronized block to avoid - // spending any more time than necessary in the synchronized section - Fqn child_fqn = new Fqn(this.fqn, child_name); - DataNode newChild = (DataNode) NodeFactory.getInstance().createNodeOfType(this, child_name, child_fqn, this, null, cache); - if (newChild == null) - throw new IllegalStateException(); - synchronized (this) - { - // check again to see if the child exists - // after acquiring exclusive lock - child = (Node) children().get(child_name); - if (child == null) - { - child = newChild; - children.put(child_name, child); - if (gtx != null) - { - MethodCall undo_op = MethodCallFactory.create(MethodDeclarations.removeNodeMethodLocal, - new Object[]{gtx, child_fqn, Boolean.FALSE}); - cache.addUndoOperation(gtx, undo_op); - // add the node name to the list maintained for the current tx - // (needed for abort/rollback of transaction) - // cache.addNode(gtx, child.getFqn()); - } - } - } - - // notify if we actually created a new child - if (newChild == child) - { - if (trace) - { - log.trace("created child: fqn=" + child_fqn); - } - cache.notifyNodeCreated(child.getFqn()); - } - } - return child; - + return getOrCreateChildWithCreateInformation(child_name, gtx, createIfNotExists).getTreeNode(); } public TreeNode createChild(Object child_name, Fqn fqn, TreeNode parent) @@ -507,13 +516,13 @@ if (trace) { log.trace(new StringBuffer("acquiring RL: fqn=").append(fqn).append(", caller=").append(caller). - append(", lock=").append(lock_.toString(DataNode.PRINT_LOCK_DETAILS))); + append(", lock=").append(lock_.toString(DataNode.PRINT_LOCK_DETAILS)).append("lock_hc="+lock_.hashCode()).append(", this._hc=").append(this.hashCode())); } boolean flag = lock_.acquireReadLock(caller, timeout); if (trace) { log.trace(new StringBuffer("acquired RL: fqn=").append(fqn).append(", caller=").append(caller). - append(", lock=").append(lock_.toString(DataNode.PRINT_LOCK_DETAILS))); + append(", lock=").append(lock_.toString(DataNode.PRINT_LOCK_DETAILS)).append("lock_hc="+lock_.hashCode()).append(", this._hc=").append(this.hashCode())); } return flag; } @@ -524,13 +533,13 @@ if (trace) { log.trace(new StringBuffer("acquiring WL: fqn=").append(fqn).append(", caller=").append(caller). - append(", lock=").append(lock_.toString(DataNode.PRINT_LOCK_DETAILS))); + append(", lock=").append(lock_.toString(DataNode.PRINT_LOCK_DETAILS)).append("lock_hc="+lock_.hashCode()).append(", this._hc=").append(this.hashCode())); } boolean flag = lock_.acquireWriteLock(caller, timeout); if (trace) { log.trace(new StringBuffer("acquired WL: fqn=").append(fqn).append(", caller=").append(caller). - append(", lock=").append(lock_.toString(DataNode.PRINT_LOCK_DETAILS))); + append(", lock=").append(lock_.toString(DataNode.PRINT_LOCK_DETAILS)).append("lock_hc="+lock_.hashCode()).append(", this._hc=").append(this.hashCode())); } return flag; } Added: core/branches/1.4.X/src/org/jboss/cache/NodeCreationResult.java =================================================================== --- core/branches/1.4.X/src/org/jboss/cache/NodeCreationResult.java (rev 0) +++ core/branches/1.4.X/src/org/jboss/cache/NodeCreationResult.java 2007-09-17 13:14:29 UTC (rev 4465) @@ -0,0 +1,19 @@ +package org.jboss.cache; + +public class NodeCreationResult +{ + private TreeNode treeNode; + private boolean created; + public boolean isCreated() { + return created; + } + public void setCreated(boolean created) { + this.created = created; + } + public TreeNode getTreeNode() { + return treeNode; + } + public void setTreeNode(TreeNode treeNode) { + this.treeNode = treeNode; + } +} Modified: core/branches/1.4.X/src/org/jboss/cache/TransactionTable.java =================================================================== --- core/branches/1.4.X/src/org/jboss/cache/TransactionTable.java 2007-09-14 17:18:10 UTC (rev 4464) +++ core/branches/1.4.X/src/org/jboss/cache/TransactionTable.java 2007-09-17 13:14:29 UTC (rev 4465) @@ -179,39 +179,45 @@ /** * Adds a lock to the global transaction. + * @return true if the lock was added */ - public void addLock(GlobalTransaction gtx, IdentityLock l) { + public boolean addLock(GlobalTransaction gtx, IdentityLock l) { TransactionEntry entry=get(gtx); if(entry == null) { log.error("transaction entry not found for (gtx=" + gtx + ")"); - return; + return false; } entry.addLock(l); + return true; } /** * Adds a collection of locks to the global transaction. + * @return true if the locks were added */ - public void addLocks(GlobalTransaction gtx, Collection locks) { + public boolean addLocks(GlobalTransaction gtx, Collection locks) { TransactionEntry entry=get(gtx); if(entry == null) { log.error("transaction entry not found for (gtx=" + gtx + ")"); - return; + return false; } entry.addLocks(locks); + return true; } /** * Adds a node that has been removed to the global transaction + * @return true if the removed node was added to the list of removed nodes */ - public void addRemovedNode(GlobalTransaction gtx, Fqn fqn) + public boolean addRemovedNode(GlobalTransaction gtx, Fqn fqn) { TransactionEntry entry=get(gtx); if(entry == null) { log.error("transaction entry not found for (gtx=" + gtx + ")"); - return; + return false; } entry.addRemovedNode(fqn); + return true; } /** Modified: core/branches/1.4.X/src/org/jboss/cache/interceptors/PessimisticLockInterceptor.java =================================================================== --- core/branches/1.4.X/src/org/jboss/cache/interceptors/PessimisticLockInterceptor.java 2007-09-14 17:18:10 UTC (rev 4464) +++ core/branches/1.4.X/src/org/jboss/cache/interceptors/PessimisticLockInterceptor.java 2007-09-17 13:14:29 UTC (rev 4465) @@ -8,11 +8,14 @@ import org.jboss.cache.DataNode; import org.jboss.cache.Fqn; +import org.jboss.cache.NodeCreationResult; import org.jboss.cache.GlobalTransaction; import org.jboss.cache.InvocationContext; +import org.jboss.cache.Node; import org.jboss.cache.TransactionEntry; import org.jboss.cache.TransactionTable; import org.jboss.cache.TreeCache; +import org.jboss.cache.TreeNode; import org.jboss.cache.lock.IdentityLock; import org.jboss.cache.lock.IsolationLevel; import org.jboss.cache.lock.LockingException; @@ -41,9 +44,8 @@ public class PessimisticLockInterceptor extends Interceptor { TransactionTable tx_table = null; - boolean writeLockOnChildInsertRemove = true; - + /** * Map<Object, java.util.List>. Keys = threads, values = lists of locks held by that thread */ @@ -115,6 +117,9 @@ lock_timeout = ((Long) args[5]).longValue(); break; case MethodDeclarations.removeNodeMethodLocal_id: + // createIfNotExists is set to true to prevent the endless loop in JBCACHE-1165. + // See http://jboss.org/index.html?module=bb&op=viewtopic&t=118186 for an explanation of this. + createIfNotExists = true; fqn = (Fqn) args[1]; lock_type = DataNode.LOCK_TYPE_WRITE; recursive = true; // remove node and *all* child nodes @@ -170,17 +175,44 @@ // release the locks for the given TX if (fqn != null) { - if (createIfNotExists) + // limit the time spent in the loop attempting to acquire locks. + long startTime = System.currentTimeMillis(); + boolean finish; + + do { - do + GlobalTransaction gtx = ctx.getGlobalTransaction(); + // first try and lock the node in question. + DataNode lockedNode = lock(fqn, gtx, lock_type, recursive, zeroLockTimeout ? 0 : lock_timeout, createIfNotExists, storeLockedNode, isRemoveData); + + // test whether we need to retry getting the lock. + finish = !createIfNotExists; + + // if we need to create the node, or the locked node was not null, we need to test if this is the *same* node that is in the cache + // to prevent a race condition between concurrent creation and deletion. + if (createIfNotExists || lockedNode != null) { - lock(fqn, ctx.getGlobalTransaction(), lock_type, recursive, zeroLockTimeout ? 0 : lock_timeout, createIfNotExists, storeLockedNode, isRemoveData); + DataNode dataNode = getNodeFromCache(fqn, ctx.getGlobalTransaction());// fetch node from cache + finish = finish || dataNode != null; + // compare the two nodes. These *should* be the same object instances. If not, we have a problem - try and re-acquire the lock. + if (dataNode != lockedNode) + { + finish = false; + if (log.isDebugEnabled()) log.debug("Lock was acquired but node changed (createIfNotExists=" + createIfNotExists + ")"); + releaseLock(lockedNode.getLock(), gtx == null ? (Object) Thread.currentThread() : gtx); + } } - while(!cache.exists(fqn)); // keep trying until we have the lock (fixes concurrent remove()) - // terminates successfully, or with (Timeout)Exception - } - else - lock(fqn, ctx.getGlobalTransaction(), lock_type, recursive, zeroLockTimeout ? 0 : lock_timeout, createIfNotExists, storeLockedNode, isRemoveData); + + // additional timeout check - if we haven't finished and have taken > lock_timeout milliseconds, throw a TimeoutException. + // if zeroLockTimeout is specified, there is no time for retries! + if (!finish && (zeroLockTimeout || System.currentTimeMillis() - startTime > lock_timeout)) + { + String msg = "PessimicticLockInterceptor can't acquire lock after [" + lock_timeout + "] ms"; + log.error(msg); + throw new TimeoutException(msg); + } + + } while (!finish); // keep trying until we have the lock, or we time out. } else { @@ -209,9 +241,44 @@ return o; } + /** + * Return node from cache, include nodes marked as removal only if they are removed by this transaction + * + * @param fqn fqn + * @return DataNode + */ + private DataNode getNodeFromCache(Fqn fqn, GlobalTransaction gtx) + { + DataNode root = cache.getRoot(); + if (fqn == null || fqn.size() == 0) return root; + TreeNode n = root; + int fqnSize = fqn.size(); + for (int i = 0; i < fqnSize; i++) + { + Object obj = fqn.get(i); + n = n.getChild(obj); + if (n == null) + return null; + else if (((DataNode) n).isMarkedForRemoval()) + { + if (gtx != null && !tx_table.get(gtx).getRemovedNodes().contains(n.getFqn())) + { + return null; + } + } + } + return (Node) n; + } + + private void cleanup(GlobalTransaction gtx) { TransactionEntry entry = tx_table.get(gtx); + if (entry == null) + { + if (log.isInfoEnabled()) log.info("Unable to clean up since transaction entry for tx " + gtx + " is null. Perhaps the transaction has timed out?"); + return; + } // Let's do it in stack style, LIFO entry.releaseAllLocksLIFO(gtx); @@ -231,10 +298,11 @@ * @param gtx * @param lock_type DataNode.LOCK_TYPE_READ, DataNode.LOCK_TYPE_WRITE or DataNode.LOCK_TYPE_NONE * @param recursive Lock children recursively + * @return node on which lock was acquired or null if lock was not acquired */ - private void lock(Fqn fqn, GlobalTransaction gtx, int lock_type, boolean recursive, - long lock_timeout, boolean createIfNotExists, boolean isRemoveNodeOperation, boolean isRemoveDataOperation) - throws TimeoutException, LockingException, InterruptedException + private DataNode lock(Fqn fqn, GlobalTransaction gtx, int lock_type, boolean recursive, + long lock_timeout, boolean createIfNotExists, boolean isRemoveNodeOperation, boolean isRemoveDataOperation) + throws TimeoutException, LockingException, InterruptedException { DataNode n; DataNode child_node; @@ -250,11 +318,11 @@ if (fqn == null) { log.error("fqn is null - this should not be the case"); - return; + return null; } if ((treeNodeSize = fqn.size()) == 0) - return; + return null; if (cache.getIsolationLevelClass() == IsolationLevel.NONE) lock_type = DataNode.LOCK_TYPE_NONE; @@ -262,6 +330,7 @@ n = cache.getRoot(); for (int i = -1; i < treeNodeSize; i++) { + boolean nodeCreated = false; if (i == -1) { child_name = Fqn.ROOT.getName(); @@ -270,14 +339,17 @@ else { child_name = fqn.get(i); - child_node = (DataNode) n.getOrCreateChild(child_name, gtx, createIfNotExists); +// child_node = (DataNode) n.getOrCreateChild(child_name, gtx, createIfNotExists); + NodeCreationResult result = n.getOrCreateChildWithCreateInformation(child_name, gtx, createIfNotExists); + child_node = (DataNode) result.getTreeNode(); + nodeCreated = result.isCreated(); } if (child_node == null) { if (log.isTraceEnabled()) log.trace("failed to find or create child " + child_name + " of node " + n.getFqn()); - return; + return null; } if (lock_type == DataNode.LOCK_TYPE_NONE) @@ -288,7 +360,7 @@ } else { - if (writeLockNeeded(lock_type, i, treeNodeSize, isRemoveNodeOperation, createIfNotExists, isRemoveDataOperation, fqn, child_node.getFqn())) + if (nodeCreated || writeLockNeeded(lock_type, i, treeNodeSize, isRemoveNodeOperation, createIfNotExists, isRemoveDataOperation, fqn, child_node.getFqn())) { currentLockType = DataNode.LOCK_TYPE_WRITE; } @@ -297,14 +369,12 @@ currentLockType = DataNode.LOCK_TYPE_READ; } } - // reverse the "remove" if the node has been previously removed in the same tx, if this operation is a put() if (gtx != null && needToReverseRemove(child_node, tx_table.get(gtx), lock_type, isRemoveNodeOperation, createIfNotExists)) { reverseRemove(child_node); } - // Try to acquire the lock; recording that we did if successful acquireNodeLock(child_node, owner, gtx, currentLockType, lock_timeout); @@ -316,7 +386,12 @@ { if (gtx != null) { - cache.getTransactionTable().addLocks(gtx, acquired_locks); + boolean reallyAdded = tx_table.addLocks(gtx, acquired_locks); + if (!reallyAdded) + { + releaseLocks(acquired_locks, owner); + throw new LockingException("Locks can't be added to current transaction table (transaction timed out?)"); + } } else { @@ -330,13 +405,21 @@ } // Add the Fqn to be removed to the transaction entry so we can clean up after ourselves during commit/rollback - if (isRemoveNodeOperation && gtx != null) cache.getTransactionTable().get(gtx).addRemovedNode(fqn); + if (isRemoveNodeOperation && gtx != null) + { + if (!tx_table.addRemovedNode(gtx, fqn)) + { + throw new LockingException("Removed node can't be added to current transaction table (transaction timed out?)"); + } + } + return n; } + private boolean needToReverseRemove(DataNode n, TransactionEntry te, int lockTypeRequested, boolean isRemoveOperation, boolean createIfNotExists) { return !isRemoveOperation && createIfNotExists && lockTypeRequested == DataNode.LOCK_TYPE_WRITE && n.isMarkedForRemoval() - && hasBeenRemovedInCurrentTx(te, n.getFqn()); + && hasBeenRemovedInCurrentTx(te, n.getFqn()); } private boolean hasBeenRemovedInCurrentTx(TransactionEntry te, Fqn f) @@ -363,11 +446,11 @@ { if (isRemoveOperation && currentNodeIndex == treeNodeSize - 2) return true; // we're doing a remove and we've reached the PARENT node of the target to be removed. - + if (!isTargetNode(currentNodeIndex, treeNodeSize) && !cache.exists(new Fqn(currentFqn, targetFqn.get(currentNodeIndex + 1)))) return isPutOperation; // we're at a node in the tree, not yet at the target node, and we need to create the next node. So we need a WL here. } - + return lock_type == DataNode.LOCK_TYPE_WRITE && isTargetNode(currentNodeIndex, treeNodeSize) && (isPutOperation || isRemoveOperation || isRemoveDataOperation); //normal operation, write lock explicitly requested and this is the target to be written to. } @@ -382,17 +465,50 @@ if (acquired) { // Record the lock for release on method return or tx commit/rollback - recordNodeLock(gtx, node.getLock()); + recordNodeLock(gtx, node.getLock(), owner); } } - private void recordNodeLock(GlobalTransaction gtx, IdentityLock lock) + private void releaseLocks(Set acquired_locks, Object owner) { + if (acquired_locks != null && !acquired_locks.isEmpty()) + { + Iterator iter = acquired_locks.iterator(); + while (iter.hasNext()) + { + IdentityLock identityLock = (IdentityLock) iter.next(); + releaseLock(identityLock, owner); + } + } + } + + private void releaseLock(IdentityLock lock, Object owner) + { + if (lock != null) + { + if (log.isDebugEnabled()) + { + log.debug("Releasing lock [" + lock + "]"); + } + lock.release(owner); + } + } + + private void recordNodeLock(GlobalTransaction gtx, IdentityLock lock, Object owner) throws LockingException + { if (gtx != null) { + // test that the gtx is associated with a valid tx. + Transaction tx = tx_table.getLocalTransaction(gtx); + if (!isValid(tx)) throw new LockingException("Locks can't be added to current transaction (transaction timed out?)"); // add the lock to the list of locks maintained for this transaction // (needed for release of locks on commit or rollback) - cache.getTransactionTable().addLock(gtx, lock); + boolean reallyAdded = tx_table.addLock(gtx, lock); + if (!reallyAdded) + { + releaseLock(lock, owner); + throw new LockingException("Locks can't be added to current transaction (transaction timed out?)"); + } } else { @@ -402,7 +518,6 @@ } } - private List getLocks(Thread currentThread) { // This sort of looks like a get/put race condition, but @@ -457,7 +572,7 @@ TransactionEntry entry = tx_table.get(gtx); if (entry == null) { - log.error("entry for transaction " + gtx + " not found (maybe already committed)"); + if (log.isInfoEnabled()) log.info("entry for transaction " + gtx + " not found (transaction timed out?)"); return; } @@ -491,7 +606,7 @@ if (entry == null) { - log.error("entry for transaction " + tx + " not found (transaction has possibly already been rolled back)"); + if (log.isInfoEnabled()) log.info("entry for transaction " + tx + " not found (transaction timed out?)"); return; } @@ -508,3 +623,4 @@ } } + Modified: core/branches/1.4.X/src/org/jboss/cache/interceptors/TxInterceptor.java =================================================================== --- core/branches/1.4.X/src/org/jboss/cache/interceptors/TxInterceptor.java 2007-09-14 17:18:10 UTC (rev 4464) +++ core/branches/1.4.X/src/org/jboss/cache/interceptors/TxInterceptor.java 2007-09-17 13:14:29 UTC (rev 4465) @@ -874,8 +874,7 @@ } else { - log.warn("Local transaction does not exist or does not match expected transaction " + gtx); - throw new CacheException(" local transaction " + ltx + " does not exist or does not match expected transaction " + gtx); + throw new CacheException(" local transaction " + ltx + " does not exist or does not match expected transaction " + gtx + " - perhaps the transaction has timed out?"); } return result; } @@ -1037,18 +1036,27 @@ public void beforeCompletion() { - if (log.isTraceEnabled()) log.trace("Running beforeCompletion on gtx " + gtx); - entry = txTable.get(gtx); - if (entry == null) - { - log.error("Transaction has a null transaction entry - beforeCompletion() will fail."); - log.error("TxTable contents: " + txTable); - throw new IllegalStateException("cannot find transaction entry for " + gtx); - } + if (log.isTraceEnabled()) log.trace("Running beforeCompletion on gtx " + gtx); + // make sure we refresh the tx from the tx_table as well, as it may have timed out. + this.tx = txTable.getLocalTransaction(gtx); - modifications = entry.getModifications(); - } + if (tx == null) + { + log.error("Transaction is null in the transaction table. Perhaps it timed out?"); + throw new IllegalStateException("Transaction is null in the transaction table. Perhaps it timed out?"); + } + entry = txTable.get(gtx); + if (entry == null) + { + log.error("Transaction has a null transaction entry - beforeCompletion() will fail."); + log.error("TxTable contents: " + txTable); + throw new IllegalStateException("cannot find transaction entry for " + gtx); + } + + modifications = entry.getModifications(); + } + // this should really not be done here - // it is supposed to be post commit not actually run the commit public void afterCompletion(int status) @@ -1155,7 +1163,7 @@ } break; default: - throw new CacheException("transaction " + tx + " in status " + tx.getStatus() + " unbale to start transaction"); + throw new CacheException("transaction " + tx + " in status " + tx.getStatus() + " unable to start transaction"); } } catch (Throwable t) Modified: core/branches/1.4.X/tests/functional/org/jboss/cache/transaction/AsyncRollbackTransactionManager.java =================================================================== --- core/branches/1.4.X/tests/functional/org/jboss/cache/transaction/AsyncRollbackTransactionManager.java 2007-09-14 17:18:10 UTC (rev 4464) +++ core/branches/1.4.X/tests/functional/org/jboss/cache/transaction/AsyncRollbackTransactionManager.java 2007-09-17 13:14:29 UTC (rev 4465) @@ -13,6 +13,7 @@ import javax.transaction.RollbackException; import javax.transaction.SystemException; import javax.transaction.Transaction; +import javax.transaction.Status; import java.util.HashMap; import java.util.Iterator; import java.util.Map; @@ -111,8 +112,19 @@ Transaction currentTx; if ((currentTx = getTransaction()) != null) { - throw new NotSupportedException(Thread.currentThread() + + switch (currentTx.getStatus()) + { + case Status.STATUS_COMMITTED: + case Status.STATUS_NO_TRANSACTION: + case Status.STATUS_ROLLEDBACK: + case Status.STATUS_UNKNOWN: + setTransaction(null); + break; + default: + throw new NotSupportedException(Thread.currentThread() + " is already associated with a transaction (" + currentTx + ")"); + } + } AsyncRollbackTransaction tx = new AsyncRollbackTransaction(this, timeout); setTransaction(tx); Modified: core/branches/1.4.X/tests/functional/org/jboss/cache/transaction/SimultaneousRollbackAndPutTest.java =================================================================== --- core/branches/1.4.X/tests/functional/org/jboss/cache/transaction/SimultaneousRollbackAndPutTest.java 2007-09-14 17:18:10 UTC (rev 4464) +++ core/branches/1.4.X/tests/functional/org/jboss/cache/transaction/SimultaneousRollbackAndPutTest.java 2007-09-17 13:14:29 UTC (rev 4465) @@ -25,7 +25,7 @@ protected void setUp() throws Exception { cache = new TreeCache(); - cache.setTransactionManagerLookupClass(DummyTransactionManagerLookup.class.getName()); + cache.setTransactionManagerLookupClass(AsyncRollbackTransactionManagerLookup.class.getName()); cache.start(); tm = cache.getTransactionManager(); cache.put(A, "k", "v"); @@ -62,7 +62,7 @@ cache.put(B, "k", "v"); // now the container should attempt to rollback the tx in a separate thread. - new Thread() + Thread rollbackThread = new Thread() { public void run() { @@ -75,21 +75,31 @@ exceptions.add(e); } } - }.start(); + }; + rollbackThread.start(); - // now try and put stuff in the main thread again - cache.put(A, "k2", "v2"); try { + // now try and put stuff in the main thread again + cache.put(A, "k2", "v2"); tm.commit(); } - catch (RollbackException expected) + catch (Exception expected) { // this is expected. } - assertEquals("No stale locks should be around", 0, cache.getNumberOfLocksHeld()); + // make sure the rollback thread has completed + rollbackThread.join(); + int nL = cache.getNumberOfLocksHeld(); + if (nL > 0) + { + System.out.println(cache.printLockInfo()); + fail("Should be no stale locks around!"); + } + //assertEquals("No stale locks should be around", 0, cache.getNumberOfLocksHeld()); + if (exceptions.size() > 0) throw ((Exception) exceptions.get(0)); } }