11:46 a.m.
Author: manik.surtani(a)jboss.com
Date: 2009-04-02 05:46:44 -0400 (Thu, 02 Apr 2009)
New Revision: 7965
Added:
core/trunk/src/test/java/org/jboss/cache/loader/AddRemoveNodeDeadlockTest.java
Modified:
core/trunk/src/main/java/org/jboss/cache/mvcc/MVCCNodeHelper.java
Log:
JBCACHE-1497 - When adding node, CacheLoaderInterceptor locks child node before
MVCCLockingInterceptor locks parent
Modified: core/trunk/src/main/java/org/jboss/cache/mvcc/MVCCNodeHelper.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/mvcc/MVCCNodeHelper.java 2009-04-02 09:29:34
UTC (rev 7964)
+++ core/trunk/src/main/java/org/jboss/cache/mvcc/MVCCNodeHelper.java 2009-04-02 09:46:44
UTC (rev 7965)
@@ -237,8 +237,14 @@
}
}
+
// see if we need to force the lock on nonexistent nodes.
- if (n == null && force) acquireLock(context, fqn);
+ if (n == null && force)
+ {
+ parentFqn = fqn.getParent();
+ if (isParentLockNeeded(parentFqn, context)) wrapNodeForWriting(context,
parentFqn, true, false, includeInvalidNodes, false, force);
+ acquireLock(context, fqn);
+ }
// now test if we need to lock the parent as well.
if ((n != null || force) && forRemoval && (parentFqn == null ?
parentFqn = fqn.getParent() : parentFqn) != null && isParentLockNeeded(parentFqn,
context))
Added: core/trunk/src/test/java/org/jboss/cache/loader/AddRemoveNodeDeadlockTest.java
===================================================================
--- core/trunk/src/test/java/org/jboss/cache/loader/AddRemoveNodeDeadlockTest.java
(rev 0)
+++
core/trunk/src/test/java/org/jboss/cache/loader/AddRemoveNodeDeadlockTest.java 2009-04-02
09:46:44 UTC (rev 7965)
@@ -0,0 +1,162 @@
+package org.jboss.cache.loader;
+
+import org.jboss.cache.AbstractSingleCacheTest;
+import org.jboss.cache.CacheSPI;
+import org.jboss.cache.Fqn;
+import org.jboss.cache.UnitTestCacheFactory;
+import org.jboss.cache.config.CacheLoaderConfig;
+import org.jboss.cache.factories.UnitTestConfigurationFactory;
+import org.jboss.cache.loader.testloaders.DummyInMemoryCacheLoader;
+import org.testng.annotations.Test;
+
+import javax.transaction.TransactionManager;
+import java.util.concurrent.Callable;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+
+@Test(groups = "functional", testName =
"loader.AddRemoveNodeDeadlockTest")
+public class AddRemoveNodeDeadlockTest extends AbstractSingleCacheTest
+{
+ private static final Fqn<String> FQN = Fqn.fromElements("a");
+ private CacheSPI<String, String> cache;
+ private TransactionManager tm;
+
+ public CacheSPI createCache() throws Exception
+ {
+ UnitTestCacheFactory<String, String> cf = new UnitTestCacheFactory<String,
String>();
+ cache = (CacheSPI<String, String>)
cf.createCache("configs/local-tx.xml", false, getClass());
+ cache.getConfiguration().setLockAcquisitionTimeout(4000);
+ cache.getConfiguration().setEvictionConfig(null);
+ // !
+ cache.getConfiguration().setLockParentForChildInsertRemove(true);
+ // !
+ CacheLoaderConfig cacheLoaderConfig =
UnitTestConfigurationFactory.buildSingleCacheLoaderConfig(false, "",
DummyInMemoryCacheLoader.class.getName(), "", false, true, false, false,
false);
+ cache.getConfiguration().setCacheLoaderConfig(cacheLoaderConfig);
+
+ cache.start();
+ tm = cache.getTransactionManager();
+
+ return cache;
+ }
+
+ private void await(CountDownLatch latch, boolean expect, String message) throws
InterruptedException, TimeoutException
+ {
+ assert latch.await(cache.getConfiguration().getLockAcquisitionTimeout() / 2,
TimeUnit.MILLISECONDS) == expect
+ : message;
+ }
+
+ public void testAdd() throws Exception
+ {
+ cache.put(FQN.getParent(), "x", "a");
+ doTest(true);
+ }
+
+ public void testRemove() throws Exception
+ {
+ cache.put(FQN, "x", "a");
+ doTest(false);
+ }
+
+ private void doTest(final boolean isAdd) throws Exception
+ {
+ ExecutorService exec = Executors.newFixedThreadPool(2);
+ try
+ {
+ final CountDownLatch init = new CountDownLatch(1);
+ final CountDownLatch parentLocked = new CountDownLatch(1);
+ final CountDownLatch childLocked = new CountDownLatch(1);
+ final CountDownLatch done = new CountDownLatch(1);
+
+ /*
+ * The procedure:
+ * 1) t1 locks the parent node
+ * 2) t2 tries to add/remove the child
+ * Under normal circumstances t2 should block on parent before adding/removing
the child.
+ * If there's a deadlock condition, it will lock the child first and then
proceed to lock the parent (and block).
+ * 3) t1 adds/removes the child
+ * Normally this should succeed as t1 already holds parent's lock.
+ * Under deadlock condition it will be blocked by t2's lock on child.
+ */
+
+ // t1
+ Future<?> f1 = exec.submit(new Callable<Void>()
+ {
+ public Void call() throws Exception
+ {
+ tm.begin();
+ try
+ {
+ await(init, true, "init t1");
+ cache.put(FQN.getParent(), "x", "b");
+ parentLocked.countDown();
+
+ // this will time out because there's no one who can ping the
latch, but that's expected
+ await(childLocked, false, "child was locked by t2");
+ cache.put(FQN, "x", "b");
+ done.countDown();
+
+ tm.commit();
+ }
+ finally
+ {
+ if (tm.getTransaction() != null)
+ {
+ tm.rollback();
+ }
+ }
+
+ return null;
+ }
+ });
+
+ // t2
+ Future<?> f2 = exec.submit(new Callable<Void>()
+ {
+ public Void call() throws Exception
+ {
+ tm.begin();
+ try
+ {
+ init.countDown();
+ await(parentLocked, true, "t2 parent lock");
+ // uncomment following line to simulate proper locking order
+// cache.put(FQN.getParent(), "x", "a");
+ if (isAdd)
+ {
+ // the deadlock exception would be here:
+ cache.put(FQN, "x", "a");
+ }
+ else
+ {
+ cache.removeNode(FQN);
+ }
+ childLocked.countDown();
+ await(done, true, "t2 done");
+
+ tm.commit();
+ }
+ finally
+ {
+ if (tm.getTransaction() != null)
+ {
+ tm.rollback();
+ }
+ }
+
+ return null;
+ }
+ });
+
+ f1.get(cache.getConfiguration().getLockAcquisitionTimeout() * 3,
TimeUnit.MILLISECONDS);
+ f2.get(cache.getConfiguration().getLockAcquisitionTimeout() * 3,
TimeUnit.MILLISECONDS);
+ }
+ finally
+ {
+ exec.shutdown();
+ }
+ }
+}