Author: manik.surtani(a)jboss.com Date: 2007-09-17 09:23:30 -0400 (Mon, 17 Sep 2007) New Revision: 4466 Removed: core/branches/1.4.X/tests/functional/org/jboss/cache/lock/pessimistic/LockNotReleasedTest.java Modified: core/branches/1.4.X/src/org/jboss/cache/interceptors/PessimisticLockInterceptor.java Log: Fixes for JBCACHE-1165, JBCACHE-1166, JBCACHE-1168, JBCACHE-1164, JBCACHE-1183 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-17 13:14:29 UTC (rev 4465) +++ core/branches/1.4.X/src/org/jboss/cache/interceptors/PessimisticLockInterceptor.java 2007-09-17 13:23:30 UTC (rev 4466) @@ -173,7 +173,7 @@ // If no TX: add each acquired lock to the list of locks for this method (locks) // If TX: [merge code from TransactionInterceptor]: register with TxManager, on commit/rollback, // release the locks for the given TX - if (fqn != null) + /*if (fqn != null) { // limit the time spent in the loop attempting to acquire locks. long startTime = System.currentTimeMillis(); @@ -218,7 +218,67 @@ { if (log.isTraceEnabled()) log.trace("bypassed locking as method " + m.getName() + "() doesn't require locking"); + }*/ + + + if (fqn != null) + { + long startTime = System.currentTimeMillis();//we can;t spend in this loop more than lock_timeout! + if (createIfNotExists) + { + boolean finish = false; + do + { + if (!zeroLockTimeout && (System.currentTimeMillis()-startTime>lock_timeout)){ + String msg = "PessimicticLockInterceptor can't acquire lock after ["+lock_timeout+"] ms."; + log.error(msg); + throw new TimeoutException(msg); + } + DataNode lockedNode = lock(fqn, ctx.getGlobalTransaction(), lock_type, recursive, zeroLockTimeout ? 0 : lock_timeout, createIfNotExists, storeLockedNode, isRemoveData); + DataNode dataNode = getNodeFromCache(fqn,ctx.getGlobalTransaction()); + finish = dataNode!=null; + if (finish && lockedNode!=null){ + if (dataNode!=lockedNode){ + finish = false; + log.trace("Lock was acquired but node changed (createIfNotExists=true)"); + } + } + } + while(!finish); // keep trying until we have the lock (fixes concurrent remove()) + // terminates successfully, or with (Timeout)Exception + } + else{ + boolean finish = false; + do + { + if (!zeroLockTimeout && (System.currentTimeMillis()-startTime>lock_timeout)){ + String msg = "PessimicticLockInterceptor can't acquire lock after ["+lock_timeout+"] ms"; + log.error(msg); + throw new TimeoutException(msg); + } + DataNode lockedNode = lock(fqn, ctx.getGlobalTransaction(), lock_type, recursive, zeroLockTimeout ? 0 : lock_timeout, createIfNotExists, storeLockedNode, isRemoveData); + if (lockedNode!=null){ + DataNode dataNode = getNodeFromCache(fqn,ctx.getGlobalTransaction()); + if (dataNode!=lockedNode){ + finish = false; + log.trace("Lock was acquired but node changed (createIfNotExists=false)"); + }else{ + finish=true; + } + }else{ + finish = true; + } + } + while(!finish); // keep trying until we have the lock (fixes concurrent remove()) + } } + else + { + if (log.isTraceEnabled()) + log.trace("bypassed locking as method " + m.getName() + "() doesn't require locking"); + } + + if (m.getMethodId() == MethodDeclarations.lockMethodLocal_id) return null; Deleted: core/branches/1.4.X/tests/functional/org/jboss/cache/lock/pessimistic/LockNotReleasedTest.java =================================================================== --- core/branches/1.4.X/tests/functional/org/jboss/cache/lock/pessimistic/LockNotReleasedTest.java 2007-09-17 13:14:29 UTC (rev 4465) +++ core/branches/1.4.X/tests/functional/org/jboss/cache/lock/pessimistic/LockNotReleasedTest.java 2007-09-17 13:23:30 UTC (rev 4466) @@ -1,175 +0,0 @@ -package org.jboss.cache.lock.pessimistic; - -import junit.framework.TestCase; -import org.jboss.cache.CacheException; -import org.jboss.cache.DummyTransactionManagerLookup; -import org.jboss.cache.Fqn; -import org.jboss.cache.Node; -import org.jboss.cache.TreeCache; -import org.jboss.cache.lock.IsolationLevel; - -import javax.transaction.TransactionManager; -import java.util.ArrayList; -import java.util.Iterator; -import java.util.List; -import java.util.Map; -import java.util.Random; - -public class LockNotReleasedTest extends TestCase -{ - Random rand = new Random(System.currentTimeMillis()); - - private TransactionManager tm; - - private TreeCache cache; - - private void startTest() throws Exception - { - cache = new TreeCache(); - cache.setCacheMode(TreeCache.LOCAL); - cache.setIsolationLevel(IsolationLevel.READ_COMMITTED); - cache.setTransactionManagerLookupClass(DummyTransactionManagerLookup.class.getName()); - cache.setLockAcquisitionTimeout(1000); - cache.create(); - cache.start(); - tm = cache.getTransactionManager(); - } - - public void tearDown() throws Exception - { - cache.stop(); - cache.destroy(); - } - - - public int getNumberOfLocksHeld() - { - return numLocks((Node) cache.getRoot()); - } - - private int numLocks(Node n) - { - int num = 0; - Map children; - - if (n.getLock().isLocked()) - { - num++; - System.out.println("Node [" + n.getFqn() + "] is locked"); - } - if ((children = n.getChildren(true)) != null) - { - for (Iterator i = children.values().iterator(); i.hasNext();) - { - num += numLocks((Node) i.next()); - } - } - return num; - } - - public void testTransactionStorm() throws Exception - { - startTest(); - List threads = new ArrayList(); -// System.out.println(cache.getNumberOfLocksHeld()); - assertEquals(0, getNumberOfLocksHeld()); - while (true) - { - for (int x = 0; x < 2; x++) - { - WorkThread t = new WorkThread(x == 1); - threads.add(t); - t.start(); - } - for (Iterator i = threads.iterator(); i.hasNext();) - { - WorkThread separateThread = (WorkThread) i.next(); - separateThread.join(); - if (separateThread.getException() != null) - { - //separateThread.getException().getMessage(); - } - } - int locksNum = getNumberOfLocksHeld(); -// System.out.println("Locks="+locksNum); - // checkpoint - if (cache.getNumberOfLocksHeld() > 0) - { - System.out.println("ERROR, locks=" + locksNum); - doDomethingOnCache(1); - } - assertEquals(0, locksNum); - } - - } - - private class WorkThread extends Thread - { - Exception e = null; - - private boolean remove; - - public WorkThread(boolean remove) - { - this.remove = remove; - } - - public Exception getException() - { - return e; - } - - public void run() - { - try - { - for (int x = 0; x < 100; x++) - { - tm.begin(); - try - { - doDomethingOnCache(x); - } - finally - { - if (x % 3 == 0) - { - tm.commit(); - } - else - { - tm.rollback(); - } - } - } - } - catch (Exception e) - { -// System.out.println(e.getMessage()); - this.e = e; - } - } - - } - - private void doDomethingOnCache(int x) throws CacheException, InterruptedException - { - cache.put(Fqn.fromString("/a/b/c/a"), "text1" + x, ""); - cache.remove(Fqn.fromString("/q/b/c/d")); - cache.put(Fqn.fromString("/a/b/c/b"), "text2" + x, ""); - cache.remove(Fqn.fromString("/q/b/c/c")); - cache.put(Fqn.fromString("/a/b/c/c"), "text3" + x, ""); - cache.remove(Fqn.fromString("/q/b/c/b")); - cache.put(Fqn.fromString("/a/b/c/d"), "text4" + x, ""); - cache.remove(Fqn.fromString("/q/b/c/a")); - cache.put(Fqn.fromString("/q/b/c/a"), "text5" + x, ""); - cache.remove(Fqn.fromString("/a/b/c/d")); - cache.put(Fqn.fromString("/q/b/c/b"), "text6" + x, ""); - cache.remove(Fqn.fromString("/a/b/c/c")); - cache.put(Fqn.fromString("/q/b/c/c"), "text7" + x, ""); - cache.remove(Fqn.fromString("/a/b/c/b")); - cache.put(Fqn.fromString("/q/b/c/d"), "text8" + x, ""); - cache.remove(Fqn.fromString("/a/b/c/a")); - } - -}