[jbosscache-commits] JBoss Cache SVN: r4465 - in core/branches/1.4.X: src/org/jboss/cache/interceptors and 1 other directories.
jbosscache-commits at lists.jboss.org
jbosscache-commits at lists.jboss.org
Mon Sep 17 09:14:29 EDT 2007
Author: manik.surtani at 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));
}
}
More information about the jbosscache-commits
mailing list