From c8accd29069c3e7e874949e71ae1518371e660f0 Mon Sep 17 00:00:00 2001 From: Gary Wu Date: Fri, 23 Oct 2015 13:40:48 -0700 Subject: [PATCH] Fix resource leaks in TransactionChainProxyTest Close TransactionChainProxy objects (AutoCloseable) that were not being closed in the test cases. Change-Id: I85b1f951545b764007bdb2e808a2438c9bd4b2b2 Signed-off-by: Gary Wu --- .../datastore/TransactionChainProxyTest.java | 231 +++++++++--------- 1 file changed, 121 insertions(+), 110 deletions(-) diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/TransactionChainProxyTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/TransactionChainProxyTest.java index 874556c0ec..8338e7083a 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/TransactionChainProxyTest.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/TransactionChainProxyTest.java @@ -21,6 +21,7 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.opendaylight.controller.cluster.datastore.TransactionType.READ_WRITE; import static org.opendaylight.controller.cluster.datastore.TransactionType.WRITE_ONLY; + import akka.actor.ActorRef; import akka.util.Timeout; import java.util.concurrent.CountDownLatch; @@ -46,8 +47,8 @@ public class TransactionChainProxyTest extends AbstractTransactionProxyTest { @Test public void testNewReadOnlyTransaction() throws Exception { - DOMStoreTransaction dst = new TransactionChainProxy(mockComponentFactory).newReadOnlyTransaction(); - Assert.assertTrue(dst instanceof DOMStoreReadTransaction); + DOMStoreTransaction dst = new TransactionChainProxy(mockComponentFactory).newReadOnlyTransaction(); + Assert.assertTrue(dst instanceof DOMStoreReadTransaction); } @@ -75,39 +76,43 @@ public class TransactionChainProxyTest extends AbstractTransactionProxyTest { } @Test - public void testTransactionChainsHaveUniqueId(){ - TransactionChainProxy one = new TransactionChainProxy(mockComponentFactory); - TransactionChainProxy two = new TransactionChainProxy(mockComponentFactory); + public void testTransactionChainsHaveUniqueId() { + try (TransactionChainProxy one = new TransactionChainProxy(mockComponentFactory)) { + try (TransactionChainProxy two = new TransactionChainProxy(mockComponentFactory)) { - Assert.assertNotEquals(one.getTransactionChainId(), two.getTransactionChainId()); + Assert.assertNotEquals(one.getTransactionChainId(), two.getTransactionChainId()); + } + } } @Test - public void testRateLimitingUsedInReadWriteTxCreation(){ - TransactionChainProxy txChainProxy = new TransactionChainProxy(mockComponentFactory); + public void testRateLimitingUsedInReadWriteTxCreation() { + try (TransactionChainProxy txChainProxy = new TransactionChainProxy(mockComponentFactory)) { - txChainProxy.newReadWriteTransaction(); + txChainProxy.newReadWriteTransaction(); - verify(mockActorContext, times(1)).acquireTxCreationPermit(); + verify(mockActorContext, times(1)).acquireTxCreationPermit(); + } } @Test - public void testRateLimitingUsedInWriteOnlyTxCreation(){ - TransactionChainProxy txChainProxy = new TransactionChainProxy(mockComponentFactory); + public void testRateLimitingUsedInWriteOnlyTxCreation() { + try (TransactionChainProxy txChainProxy = new TransactionChainProxy(mockComponentFactory)) { - txChainProxy.newWriteOnlyTransaction(); + txChainProxy.newWriteOnlyTransaction(); - verify(mockActorContext, times(1)).acquireTxCreationPermit(); + verify(mockActorContext, times(1)).acquireTxCreationPermit(); + } } - @Test - public void testRateLimitingNotUsedInReadOnlyTxCreation(){ - TransactionChainProxy txChainProxy = new TransactionChainProxy(mockComponentFactory); + public void testRateLimitingNotUsedInReadOnlyTxCreation() { + try (TransactionChainProxy txChainProxy = new TransactionChainProxy(mockComponentFactory)) { - txChainProxy.newReadOnlyTransaction(); + txChainProxy.newReadOnlyTransaction(); - verify(mockActorContext, times(0)).acquireTxCreationPermit(); + verify(mockActorContext, times(0)).acquireTxCreationPermit(); + } } /** @@ -118,64 +123,66 @@ public class TransactionChainProxyTest extends AbstractTransactionProxyTest { public void testChainedWriteOnlyTransactions() throws Exception { dataStoreContextBuilder.writeOnlyTransactionOptimizationsEnabled(true); - TransactionChainProxy txChainProxy = new TransactionChainProxy(mockComponentFactory); + try (TransactionChainProxy txChainProxy = new TransactionChainProxy(mockComponentFactory)) { - ActorRef txActorRef1 = setupActorContextWithoutInitialCreateTransaction(getSystem()); + ActorRef txActorRef1 = setupActorContextWithoutInitialCreateTransaction(getSystem()); - Promise batchedReplyPromise1 = akka.dispatch.Futures.promise(); - doReturn(batchedReplyPromise1.future()).when(mockActorContext).executeOperationAsync( - eq(actorSelection(txActorRef1)), isA(BatchedModifications.class)); + Promise batchedReplyPromise1 = akka.dispatch.Futures.promise(); + doReturn(batchedReplyPromise1.future()).when(mockActorContext).executeOperationAsync( + eq(actorSelection(txActorRef1)), isA(BatchedModifications.class)); - DOMStoreWriteTransaction writeTx1 = txChainProxy.newWriteOnlyTransaction(); + DOMStoreWriteTransaction writeTx1 = txChainProxy.newWriteOnlyTransaction(); - NormalizedNode writeNode1 = ImmutableNodes.containerNode(TestModel.TEST_QNAME); - writeTx1.write(TestModel.TEST_PATH, writeNode1); + NormalizedNode writeNode1 = ImmutableNodes.containerNode(TestModel.TEST_QNAME); + writeTx1.write(TestModel.TEST_PATH, writeNode1); - writeTx1.ready(); + writeTx1.ready(); - verify(mockActorContext, times(1)).findPrimaryShardAsync(eq(DefaultShardStrategy.DEFAULT_SHARD)); + verify(mockActorContext, times(1)).findPrimaryShardAsync(eq(DefaultShardStrategy.DEFAULT_SHARD)); - verifyOneBatchedModification(txActorRef1, new WriteModification(TestModel.TEST_PATH, writeNode1), true); + verifyOneBatchedModification(txActorRef1, new WriteModification(TestModel.TEST_PATH, writeNode1), true); - ActorRef txActorRef2 = setupActorContextWithoutInitialCreateTransaction(getSystem()); + ActorRef txActorRef2 = setupActorContextWithoutInitialCreateTransaction(getSystem()); - expectBatchedModifications(txActorRef2, 1); + expectBatchedModifications(txActorRef2, 1); - final NormalizedNode writeNode2 = ImmutableNodes.containerNode(TestModel.OUTER_LIST_QNAME); + final NormalizedNode writeNode2 = ImmutableNodes.containerNode(TestModel.OUTER_LIST_QNAME); - final DOMStoreWriteTransaction writeTx2 = txChainProxy.newWriteOnlyTransaction(); + final DOMStoreWriteTransaction writeTx2 = txChainProxy.newWriteOnlyTransaction(); - final AtomicReference caughtEx = new AtomicReference<>(); - final CountDownLatch write2Complete = new CountDownLatch(1); - new Thread() { - @Override - public void run() { - try { - writeTx2.write(TestModel.OUTER_LIST_PATH, writeNode2); - } catch (Exception e) { - caughtEx.set(e); - } finally { - write2Complete.countDown(); + final AtomicReference caughtEx = new AtomicReference<>(); + final CountDownLatch write2Complete = new CountDownLatch(1); + new Thread() { + @Override + public void run() { + try { + writeTx2.write(TestModel.OUTER_LIST_PATH, writeNode2); + } catch (Exception e) { + caughtEx.set(e); + } finally { + write2Complete.countDown(); + } } - } - }.start(); + }.start(); - assertEquals("Tx 2 write should've completed", true, write2Complete.await(5, TimeUnit.SECONDS)); + assertEquals("Tx 2 write should've completed", true, write2Complete.await(5, TimeUnit.SECONDS)); - if(caughtEx.get() != null) { - throw caughtEx.get(); - } + if (caughtEx.get() != null) { + throw caughtEx.get(); + } - try { - verify(mockActorContext, times(1)).findPrimaryShardAsync(eq(DefaultShardStrategy.DEFAULT_SHARD)); - } catch (AssertionError e) { - fail("Tx 2 should not have initiated until the Tx 1's ready future completed"); - } + try { + verify(mockActorContext, times(1)).findPrimaryShardAsync(eq(DefaultShardStrategy.DEFAULT_SHARD)); + } catch (AssertionError e) { + fail("Tx 2 should not have initiated until the Tx 1's ready future completed"); + } - batchedReplyPromise1.success(readyTxReply(txActorRef1.path().toString()).value().get().get()); + batchedReplyPromise1.success(readyTxReply(txActorRef1.path().toString()).value().get().get()); - // Tx 2 should've proceeded to find the primary shard. - verify(mockActorContext, timeout(5000).times(2)).findPrimaryShardAsync(eq(DefaultShardStrategy.DEFAULT_SHARD)); + // Tx 2 should've proceeded to find the primary shard. + verify(mockActorContext, timeout(5000).times(2)).findPrimaryShardAsync( + eq(DefaultShardStrategy.DEFAULT_SHARD)); + } } /** @@ -184,85 +191,89 @@ public class TransactionChainProxyTest extends AbstractTransactionProxyTest { */ @Test public void testChainedReadWriteTransactions() throws Exception { - TransactionChainProxy txChainProxy = new TransactionChainProxy(mockComponentFactory); + try (TransactionChainProxy txChainProxy = new TransactionChainProxy(mockComponentFactory)) { - ActorRef txActorRef1 = setupActorContextWithInitialCreateTransaction(getSystem(), READ_WRITE); + ActorRef txActorRef1 = setupActorContextWithInitialCreateTransaction(getSystem(), READ_WRITE); - expectBatchedModifications(txActorRef1, 1); + expectBatchedModifications(txActorRef1, 1); - Promise readyReplyPromise1 = akka.dispatch.Futures.promise(); - doReturn(readyReplyPromise1.future()).when(mockActorContext).executeOperationAsync( - eq(actorSelection(txActorRef1)), isA(BatchedModifications.class)); + Promise readyReplyPromise1 = akka.dispatch.Futures.promise(); + doReturn(readyReplyPromise1.future()).when(mockActorContext).executeOperationAsync( + eq(actorSelection(txActorRef1)), isA(BatchedModifications.class)); - DOMStoreWriteTransaction writeTx1 = txChainProxy.newReadWriteTransaction(); + DOMStoreWriteTransaction writeTx1 = txChainProxy.newReadWriteTransaction(); - NormalizedNode writeNode1 = ImmutableNodes.containerNode(TestModel.TEST_QNAME); - writeTx1.write(TestModel.TEST_PATH, writeNode1); + NormalizedNode writeNode1 = ImmutableNodes.containerNode(TestModel.TEST_QNAME); + writeTx1.write(TestModel.TEST_PATH, writeNode1); - writeTx1.ready(); + writeTx1.ready(); - verifyOneBatchedModification(txActorRef1, new WriteModification(TestModel.TEST_PATH, writeNode1), true); + verifyOneBatchedModification(txActorRef1, new WriteModification(TestModel.TEST_PATH, writeNode1), true); - String tx2MemberName = "mock-member"; - ActorRef shardActorRef2 = setupActorContextWithoutInitialCreateTransaction(getSystem()); - ActorRef txActorRef2 = setupActorContextWithInitialCreateTransaction(getSystem(), READ_WRITE, - DataStoreVersions.CURRENT_VERSION, tx2MemberName, shardActorRef2); + String tx2MemberName = "mock-member"; + ActorRef shardActorRef2 = setupActorContextWithoutInitialCreateTransaction(getSystem()); + ActorRef txActorRef2 = setupActorContextWithInitialCreateTransaction(getSystem(), READ_WRITE, + DataStoreVersions.CURRENT_VERSION, tx2MemberName, shardActorRef2); - expectBatchedModifications(txActorRef2, 1); + expectBatchedModifications(txActorRef2, 1); - final NormalizedNode writeNode2 = ImmutableNodes.containerNode(TestModel.OUTER_LIST_QNAME); + final NormalizedNode writeNode2 = ImmutableNodes.containerNode(TestModel.OUTER_LIST_QNAME); - final DOMStoreWriteTransaction writeTx2 = txChainProxy.newReadWriteTransaction(); + final DOMStoreWriteTransaction writeTx2 = txChainProxy.newReadWriteTransaction(); - final AtomicReference caughtEx = new AtomicReference<>(); - final CountDownLatch write2Complete = new CountDownLatch(1); - new Thread() { - @Override - public void run() { - try { - writeTx2.write(TestModel.OUTER_LIST_PATH, writeNode2); - } catch (Exception e) { - caughtEx.set(e); - } finally { - write2Complete.countDown(); + final AtomicReference caughtEx = new AtomicReference<>(); + final CountDownLatch write2Complete = new CountDownLatch(1); + new Thread() { + @Override + public void run() { + try { + writeTx2.write(TestModel.OUTER_LIST_PATH, writeNode2); + } catch (Exception e) { + caughtEx.set(e); + } finally { + write2Complete.countDown(); + } } - } - }.start(); + }.start(); - assertEquals("Tx 2 write should've completed", true, write2Complete.await(5, TimeUnit.SECONDS)); + assertEquals("Tx 2 write should've completed", true, write2Complete.await(5, TimeUnit.SECONDS)); - if(caughtEx.get() != null) { - throw caughtEx.get(); - } + if (caughtEx.get() != null) { + throw caughtEx.get(); + } - try { - verify(mockActorContext, never()).executeOperationAsync(eq(getSystem().actorSelection(shardActorRef2.path())), - eqCreateTransaction(tx2MemberName, READ_WRITE)); - } catch (AssertionError e) { - fail("Tx 2 should not have initiated until the Tx 1's ready future completed"); - } + try { + verify(mockActorContext, never()).executeOperationAsync( + eq(getSystem().actorSelection(shardActorRef2.path())), + eqCreateTransaction(tx2MemberName, READ_WRITE)); + } catch (AssertionError e) { + fail("Tx 2 should not have initiated until the Tx 1's ready future completed"); + } - readyReplyPromise1.success(readyTxReply(txActorRef1.path().toString()).value().get().get()); + readyReplyPromise1.success(readyTxReply(txActorRef1.path().toString()).value().get().get()); - verify(mockActorContext, timeout(5000)).executeOperationAsync(eq(getSystem().actorSelection(shardActorRef2.path())), - eqCreateTransaction(tx2MemberName, READ_WRITE), any(Timeout.class)); + verify(mockActorContext, timeout(5000)).executeOperationAsync( + eq(getSystem().actorSelection(shardActorRef2.path())), + eqCreateTransaction(tx2MemberName, READ_WRITE), any(Timeout.class)); + } } - @Test(expected=IllegalStateException.class) + @Test(expected = IllegalStateException.class) public void testChainedWriteTransactionsWithPreviousTxNotReady() throws Exception { ActorRef actorRef = setupActorContextWithInitialCreateTransaction(getSystem(), WRITE_ONLY); expectBatchedModifications(actorRef, 1); - TransactionChainProxy txChainProxy = new TransactionChainProxy(mockComponentFactory); + try (TransactionChainProxy txChainProxy = new TransactionChainProxy(mockComponentFactory)) { - DOMStoreWriteTransaction writeTx1 = txChainProxy.newWriteOnlyTransaction(); + DOMStoreWriteTransaction writeTx1 = txChainProxy.newWriteOnlyTransaction(); - NormalizedNode writeNode1 = ImmutableNodes.containerNode(TestModel.TEST_QNAME); - writeTx1.write(TestModel.TEST_PATH, writeNode1); + NormalizedNode writeNode1 = ImmutableNodes.containerNode(TestModel.TEST_QNAME); + writeTx1.write(TestModel.TEST_PATH, writeNode1); - NormalizedNode writeNode2 = ImmutableNodes.containerNode(TestModel.OUTER_LIST_QNAME); + NormalizedNode writeNode2 = ImmutableNodes.containerNode(TestModel.OUTER_LIST_QNAME); - txChainProxy.newWriteOnlyTransaction(); + txChainProxy.newWriteOnlyTransaction(); + } } } -- 2.36.6