Author: manik.surtani(a)jboss.com Date: 2007-09-25 12:07:25 -0400 (Tue, 25 Sep 2007) New Revision: 4505 Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/PessimisticLockInterceptor.java core/trunk/src/test/java/org/jboss/cache/transaction/IsolationLevelReadCommittedNodeCreationRollbackTest.java Log: JBCACHE-407 - Write locks not acquired on implicit node creation Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/PessimisticLockInterceptor.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/interceptors/PessimisticLockInterceptor.java 2007-09-25 13:57:00 UTC (rev 4504) +++ core/trunk/src/main/java/org/jboss/cache/interceptors/PessimisticLockInterceptor.java 2007-09-25 16:07:25 UTC (rev 4505) @@ -225,8 +225,8 @@ private long getLockAcquisitionTimeout(InvocationContext ctx) { long timeout = lock_acquisition_timeout; - if (ctx.getOptionOverrides() != null - && ctx.getOptionOverrides().getLockAcquisitionTimeout() >= 0) + if (ctx.getOptionOverrides() != null + && ctx.getOptionOverrides().getLockAcquisitionTimeout() >= 0) { timeout = ctx.getOptionOverrides().getLockAcquisitionTimeout(); } @@ -240,7 +240,7 @@ // node should have a WL. long timeout = getLockAcquisitionTimeout(ctx); - + // this call will ensure the node gets a WL and it's current parent gets RL. if (log.isTraceEnabled()) log.trace("Attempting to get WL on node to be moved [" + node + "]"); lock(ctx, node, NodeLock.LockType.WRITE, true, false, timeout, true, false, false); @@ -298,9 +298,11 @@ child_node = n.getChildDirect(child_name); } + boolean created = false; if (child_node == null && createIfNotExists) { child_node = n.addChildDirect(new Fqn(child_name)); + created = true; } if (child_node == null) @@ -320,7 +322,7 @@ } else { - if (writeLockNeeded(ctx, lock_type, i, treeNodeSize, isEvictionOperation, isDeleteOperation, createIfNotExists, isRemoveDataOperation, fqn, child_node)) + if (created || writeLockNeeded(ctx, lock_type, i, treeNodeSize, isEvictionOperation, isDeleteOperation, createIfNotExists, isRemoveDataOperation, fqn, child_node)) { lockTypeRequired = NodeLock.LockType.WRITE; Modified: core/trunk/src/test/java/org/jboss/cache/transaction/IsolationLevelReadCommittedNodeCreationRollbackTest.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/transaction/IsolationLevelReadCommittedNodeCreationRollbackTest.java 2007-09-25 13:57:00 UTC (rev 4504) +++ core/trunk/src/test/java/org/jboss/cache/transaction/IsolationLevelReadCommittedNodeCreationRollbackTest.java 2007-09-25 16:07:25 UTC (rev 4505) @@ -6,19 +6,7 @@ */ package org.jboss.cache.transaction; -import static org.testng.AssertJUnit.assertEquals; -import static org.testng.AssertJUnit.assertNull; -import static org.testng.AssertJUnit.fail; - -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.TimeUnit; - -import javax.transaction.NotSupportedException; -import javax.transaction.SystemException; -import javax.transaction.Transaction; - import junit.framework.AssertionFailedError; - import org.jboss.cache.CacheFactory; import org.jboss.cache.CacheImpl; import org.jboss.cache.DefaultCacheFactory; @@ -26,17 +14,24 @@ import org.jboss.cache.config.Configuration; import org.jboss.cache.lock.IsolationLevel; import org.jboss.cache.lock.TimeoutException; +import static org.testng.AssertJUnit.*; import org.testng.annotations.AfterMethod; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; +import javax.transaction.NotSupportedException; +import javax.transaction.SystemException; +import javax.transaction.Transaction; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + /** * Tests READ_COMMITED isolation level. * * @author <a href="mailto:ovidiu@jboss.org">Ovidiu Feodorov</a> * @version $Id$ */ -@Test(groups = { "functional", "transaction" }) +@Test(groups = {"functional", "transaction"}) public class IsolationLevelReadCommittedNodeCreationRollbackTest { @@ -60,7 +55,7 @@ readerError = null; CacheFactory<String, String> instance = DefaultCacheFactory.getInstance(); - cache = (CacheImpl<String, String>)instance.createCache(false); + cache = (CacheImpl<String, String>) instance.createCache(false); cache.getConfiguration().setCacheMode(Configuration.CacheMode.LOCAL); cache.getConfiguration().setIsolationLevel(IsolationLevel.READ_COMMITTED); cache.getConfiguration().setTransactionManagerLookupClass(DummyTransactionManagerLookup.class.getName()); @@ -88,7 +83,8 @@ // start a first thread and a transaction - Thread firstThread = new Thread(new Runnable() { + Thread firstThread = new Thread(new Runnable() + { public void run() { try @@ -138,7 +134,8 @@ // start a second thread; no transaction is necessary here - Thread secondThread = new Thread(new Runnable() { + Thread secondThread = new Thread(new Runnable() + { public void run() { try @@ -168,7 +165,7 @@ secondCanRead.await(); // I should still see the value I put - assertEquals("Known issue JBCACHE-407 -- write lock not acquired on " + "creation of an empty node", VALUE, cache.get(a2, KEY)); + assertEquals("write lock not acquired on " + "creation of an empty node", VALUE, cache.get(a2, KEY)); } catch (AssertionFailedError e) {