Altered test to make it expose problem. Needed to add debugUnhandledErrorListener for this
Project: http://git-wip-us.apache.org/repos/asf/curator/repo Commit: http://git-wip-us.apache.org/repos/asf/curator/commit/ccac7baa Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/ccac7baa Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/ccac7baa Branch: refs/heads/master Commit: ccac7baac38b9fe3dc5b322639ea409f7fa0f2b6 Parents: a8a3e14 Author: randgalt <randg...@apache.org> Authored: Mon Jul 28 19:00:12 2014 -0500 Committer: randgalt <randg...@apache.org> Committed: Mon Jul 28 19:00:12 2014 -0500 ---------------------------------------------------------------------- .../framework/imps/CuratorFrameworkImpl.java | 8 +- .../framework/imps/TestFrameworkBackground.java | 111 ++++++++----------- 2 files changed, 53 insertions(+), 66 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/curator/blob/ccac7baa/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 7f7cc98..01cacee 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 @@ -57,7 +57,6 @@ import java.util.concurrent.atomic.AtomicReference; public class CuratorFrameworkImpl implements CuratorFramework { - private final Logger log = LoggerFactory.getLogger(getClass()); private final CuratorZookeeperClient client; private final ListenerContainer<CuratorListener> listeners; @@ -86,6 +85,7 @@ public class CuratorFrameworkImpl implements CuratorFramework } volatile DebugBackgroundListener debugListener = null; + volatile UnhandledErrorListener debugUnhandledErrorListener = null; private final AtomicReference<CuratorFrameworkState> state; @@ -313,6 +313,7 @@ public class CuratorFrameworkImpl implements CuratorFramework Thread.currentThread().interrupt(); } } + listeners.clear(); unhandledErrorListeners.clear(); connectionStateManager.close(); @@ -566,6 +567,11 @@ public class CuratorFrameworkImpl implements CuratorFramework return null; } }); + + if ( debugUnhandledErrorListener != null ) + { + debugUnhandledErrorListener.unhandledError(reason, e); + } } String unfixForNamespace(String path) http://git-wip-us.apache.org/repos/asf/curator/blob/ccac7baa/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 f9fea4f..3f1c41f 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 @@ -20,30 +20,26 @@ package org.apache.curator.framework.imps; import com.google.common.collect.Lists; - -import org.apache.curator.test.BaseClassForTests; -import org.apache.curator.utils.CloseableUtils; import org.apache.curator.framework.CuratorFramework; import org.apache.curator.framework.CuratorFrameworkFactory; import org.apache.curator.framework.api.BackgroundCallback; import org.apache.curator.framework.api.CuratorEvent; import org.apache.curator.framework.api.UnhandledErrorListener; -import org.apache.curator.framework.imps.OperationAndData.ErrorCallback; import org.apache.curator.framework.state.ConnectionState; import org.apache.curator.framework.state.ConnectionStateListener; import org.apache.curator.retry.RetryNTimes; import org.apache.curator.retry.RetryOneTime; +import org.apache.curator.test.BaseClassForTests; import org.apache.curator.test.TestingServer; import org.apache.curator.test.Timing; +import org.apache.curator.utils.CloseableUtils; import org.apache.zookeeper.KeeperException.Code; import org.testng.Assert; import org.testng.annotations.Test; - import java.util.Arrays; import java.util.List; import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; @@ -68,7 +64,8 @@ public class TestFrameworkBackground extends BaseClassForTests @Override public void stateChanged(CuratorFramework client, ConnectionState newState) { - if ( firstListenerAction.compareAndSet(true, false) ) { + if ( firstListenerAction.compareAndSet(true, false) ) + { firstListenerState.set(newState); System.out.println("First listener state is " + newState); } @@ -185,11 +182,7 @@ public class TestFrameworkBackground extends BaseClassForTests public void testCuratorCallbackOnError() throws Exception { Timing timing = new Timing(); - CuratorFramework client = CuratorFrameworkFactory.builder() - .connectString(server.getConnectString()) - .sessionTimeoutMs(timing.session()) - .connectionTimeoutMs(timing.connection()) - .retryPolicy(new RetryOneTime(1000)).build(); + CuratorFramework client = CuratorFrameworkFactory.builder().connectString(server.getConnectString()).sessionTimeoutMs(timing.session()).connectionTimeoutMs(timing.connection()).retryPolicy(new RetryOneTime(1000)).build(); final CountDownLatch latch = new CountDownLatch(1); try { @@ -198,8 +191,7 @@ public class TestFrameworkBackground extends BaseClassForTests { @Override - public void processResult(CuratorFramework client, CuratorEvent event) - throws Exception + public void processResult(CuratorFramework client, CuratorEvent event) throws Exception { if ( event.getResultCode() == Code.CONNECTIONLOSS.intValue() ) { @@ -220,7 +212,7 @@ public class TestFrameworkBackground extends BaseClassForTests } } - + /** * CURATOR-126 * Shutdown the Curator client while there are still background operations running. @@ -228,77 +220,66 @@ public class TestFrameworkBackground extends BaseClassForTests @Test public void testShutdown() throws Exception { - final int MAX_CLOSE_WAIT_MS = 5000; Timing timing = new Timing(); - CuratorFramework client = CuratorFrameworkFactory.builder().connectString(server.getConnectString()).sessionTimeoutMs(timing.session()). - connectionTimeoutMs(timing.connection()).retryPolicy(new RetryOneTime(1)).maxCloseWaitMs(MAX_CLOSE_WAIT_MS).build(); + CuratorFramework client = CuratorFrameworkFactory + .builder() + .connectString(server.getConnectString()) + .sessionTimeoutMs(timing.session()) + .connectionTimeoutMs(timing.connection()).retryPolicy(new RetryOneTime(1)) + .maxCloseWaitMs(timing.forWaiting().milliseconds()) + .build(); try { - client.start(); - - BackgroundCallback callback = new BackgroundCallback() + final AtomicBoolean hadIllegalStateException = new AtomicBoolean(false); + ((CuratorFrameworkImpl)client).debugUnhandledErrorListener = new UnhandledErrorListener() { @Override - public void processResult(CuratorFramework client, CuratorEvent event) throws Exception - { + public void unhandledError(String message, Throwable e) + { + if ( e instanceof IllegalStateException ) + { + hadIllegalStateException.set(true); + } } }; - + client.start(); + final CountDownLatch operationReadyLatch = new CountDownLatch(1); - - //This gets called just before the operation is run. ((CuratorFrameworkImpl)client).debugListener = new CuratorFrameworkImpl.DebugBackgroundListener() { @Override public void listen(OperationAndData<?> data) { - operationReadyLatch.countDown(); - - try { - Thread.sleep(MAX_CLOSE_WAIT_MS / 2); - } catch(InterruptedException e) { + try + { + operationReadyLatch.await(); + } + catch ( InterruptedException e ) + { + Thread.currentThread().interrupt(); } - } - }; - - Assert.assertTrue(client.getZookeeperClient().blockUntilConnectedOrTimedOut(), "Failed to connect"); - - server.stop(); - - BackgroundOperation<String> background = new BackgroundOperation<String>() - { - - @Override - public void performBackgroundOperation(OperationAndData<String> data) - throws Exception - { } }; - - ErrorCallback<String> errorCallback = new ErrorCallback<String>() - { - @Override - public void retriesExhausted( - OperationAndData<String> operationAndData) - { - } - }; - - OperationAndData<String> operation = new OperationAndData<String>(background, - "thedata", callback, errorCallback, null); - - ((CuratorFrameworkImpl)client).queueOperation(operation); - - operationReadyLatch.await(); - + // queue a background operation that will block due to the debugListener + client.create().inBackground().forPath("/hey"); + timing.sleepABit(); + + // close the client while the background is still blocked client.close(); + + // unblock the background + operationReadyLatch.countDown(); + timing.sleepABit(); + + // should not generate an exception + Assert.assertFalse(hadIllegalStateException.get()); } finally { CloseableUtils.closeQuietly(client); - } + } } - - + + }