Repository: curator Updated Branches: refs/heads/CURATOR-106 [created] ec4d74a78
CURATOR-106: Ensure background operations happen in the background thread to avoid stack overflows when retrying guaranteed operations Project: http://git-wip-us.apache.org/repos/asf/curator/repo Commit: http://git-wip-us.apache.org/repos/asf/curator/commit/757e45f6 Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/757e45f6 Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/757e45f6 Branch: refs/heads/CURATOR-106 Commit: 757e45f6d9ff6a471642616daf0668d8a64c4324 Parents: 302661a Author: Stig Rohde Døssing <[email protected]> Authored: Fri Apr 21 22:03:27 2017 +0200 Committer: Stig Rohde Døssing <[email protected]> Committed: Fri Apr 21 23:18:58 2017 +0200 ---------------------------------------------------------------------- curator-framework/pom.xml | 6 +++ .../framework/imps/CuratorFrameworkImpl.java | 2 +- .../framework/imps/TestFrameworkBackground.java | 42 ++++++++++++++++++++ .../nodes/TestPersistentEphemeralNode.java | 4 ++ 4 files changed, 53 insertions(+), 1 deletion(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/curator/blob/757e45f6/curator-framework/pom.xml ---------------------------------------------------------------------- diff --git a/curator-framework/pom.xml b/curator-framework/pom.xml index 9080308..2860658 100644 --- a/curator-framework/pom.xml +++ b/curator-framework/pom.xml @@ -67,6 +67,12 @@ <artifactId>slf4j-log4j12</artifactId> <scope>test</scope> </dependency> + + <dependency> + <groupId>org.mockito</groupId> + <artifactId>mockito-core</artifactId> + <scope>test</scope> + </dependency> </dependencies> </project> http://git-wip-us.apache.org/repos/asf/curator/blob/757e45f6/curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java ---------------------------------------------------------------------- diff --git a/curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java b/curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java index 6aa9f66..545b7a4 100644 --- a/curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java +++ b/curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java @@ -504,7 +504,7 @@ public class CuratorFrameworkImpl implements CuratorFramework boolean isInitialExecution = (event == null); if ( isInitialExecution ) { - performBackgroundOperation(operationAndData); + queueOperation(operationAndData); return; } http://git-wip-us.apache.org/repos/asf/curator/blob/757e45f6/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFrameworkBackground.java ---------------------------------------------------------------------- diff --git a/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFrameworkBackground.java b/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFrameworkBackground.java index 8e21929..50cb006 100644 --- a/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFrameworkBackground.java +++ b/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFrameworkBackground.java @@ -40,9 +40,13 @@ import org.testng.annotations.Test; import java.util.Arrays; import java.util.List; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; +import org.mockito.Mockito; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; public class TestFrameworkBackground extends BaseClassForTests { @@ -328,6 +332,44 @@ public class TestFrameworkBackground extends BaseClassForTests CloseableUtils.closeQuietly(client); } } + + /** + * CURATOR-106: Background operations must always run in the background to avoid potential stack overflows when doing guaranteed operations + */ + @Test + public void testBackgroundOperationsAreExecutedInTheBackground() throws Exception { + Timing timing = new Timing(); + CuratorFramework client = CuratorFrameworkFactory.newClient(server.getConnectString(), timing.session(), timing.connection(), new RetryOneTime(1)); + try + { + client.start(); + client.blockUntilConnected(30, TimeUnit.SECONDS); + + final CountDownLatch latch = new CountDownLatch(1); + final AtomicReference<Thread> backgroundThread = new AtomicReference<Thread>(); + + OperationAndData<Void> operation = (OperationAndData<Void>)Mockito.mock(OperationAndData.class); + Mockito.doAnswer(new Answer() { + @Override + public Void answer(InvocationOnMock invocation) throws Throwable { + backgroundThread.set(Thread.currentThread()); + latch.countDown(); + return null; + } + }).when(operation).callPerformBackgroundOperation(); + + ((CuratorFrameworkImpl)client).processBackgroundOperation(operation, null); + + latch.await(); + + Assert.assertNotNull(backgroundThread.get()); + Assert.assertNotEquals(Thread.currentThread(), backgroundThread.get()); + } + finally + { + CloseableUtils.closeQuietly(client); + } + } } http://git-wip-us.apache.org/repos/asf/curator/blob/757e45f6/curator-recipes/src/test/java/org/apache/curator/framework/recipes/nodes/TestPersistentEphemeralNode.java ---------------------------------------------------------------------- diff --git a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/nodes/TestPersistentEphemeralNode.java b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/nodes/TestPersistentEphemeralNode.java index ef8a45a..4855876 100644 --- a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/nodes/TestPersistentEphemeralNode.java +++ b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/nodes/TestPersistentEphemeralNode.java @@ -633,7 +633,11 @@ public class TestPersistentEphemeralNode extends BaseClassForTests node.waitForInitialCreate(timing.forWaiting().seconds(), TimeUnit.SECONDS); assertTrue(Arrays.equals(curator.getData().forPath(node.getActualPath()), initialData)); + Trigger dataChangedTrigger = Trigger.dataChanged(); + curator.getData().usingWatcher(dataChangedTrigger).forPath(node.getActualPath()); + node.setData(updatedData); + assertTrue(dataChangedTrigger.firedWithin(timing.forWaiting().seconds(), TimeUnit.SECONDS)); assertTrue(Arrays.equals(curator.getData().forPath(node.getActualPath()), updatedData)); server.restart();
