curator-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From cammcken...@apache.org
Subject [1/4] git commit: CURATOR-141 Don't make calls to the client after PathChildrenCache has been closed.
Date Wed, 20 Aug 2014 05:54:56 GMT
Repository: curator
Updated Branches:
  refs/heads/CURATOR-141 [created] 0d1397af1


CURATOR-141 Don't make calls to the client after PathChildrenCache has been closed.

The test case is a bit weird because it needed to intercept logging.
Also, I've added slf4j-log4j12 as a test dependency to control logging.
I think it should be added to the other poms as well, otherwise logging
seems to get lost.


Project: http://git-wip-us.apache.org/repos/asf/curator/repo
Commit: http://git-wip-us.apache.org/repos/asf/curator/commit/f2f1953c
Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/f2f1953c
Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/f2f1953c

Branch: refs/heads/CURATOR-141
Commit: f2f1953c544398cb6db53a2ac59b02719cc83fa3
Parents: d2c37d0
Author: Karel Vervaeke <karel@ngdata.com>
Authored: Thu Aug 14 13:46:03 2014 +0200
Committer: Karel Vervaeke <karel@ngdata.com>
Committed: Thu Aug 14 13:46:03 2014 +0200

----------------------------------------------------------------------
 curator-recipes/pom.xml                         |  6 ++
 .../recipes/cache/PathChildrenCache.java        |  4 +
 .../recipes/cache/TestPathChildrenCache.java    | 85 +++++++++++++++++++-
 pom.xml                                         |  6 ++
 4 files changed, 100 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/curator/blob/f2f1953c/curator-recipes/pom.xml
----------------------------------------------------------------------
diff --git a/curator-recipes/pom.xml b/curator-recipes/pom.xml
index 13aa475..37155c9 100644
--- a/curator-recipes/pom.xml
+++ b/curator-recipes/pom.xml
@@ -61,5 +61,11 @@
             <artifactId>mockito-core</artifactId>
             <scope>test</scope>
         </dependency>
+
+        <dependency>
+            <groupId>org.slf4j</groupId>
+            <artifactId>slf4j-log4j12</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 </project>

http://git-wip-us.apache.org/repos/asf/curator/blob/f2f1953c/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/PathChildrenCache.java
----------------------------------------------------------------------
diff --git a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/PathChildrenCache.java
b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/PathChildrenCache.java
index d555508..57c6e92 100644
--- a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/PathChildrenCache.java
+++ b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/PathChildrenCache.java
@@ -487,6 +487,10 @@ public class PathChildrenCache implements Closeable
             @Override
             public void processResult(CuratorFramework client, CuratorEvent event) throws
Exception
             {
+                if (PathChildrenCache.this.state.get().equals(State.CLOSED)) {
+                    // This ship is closed, don't handle the callback
+                    return;
+                }
                 if ( event.getResultCode() == KeeperException.Code.OK.intValue() )
                 {
                     processChildren(event.getChildren(), mode);

http://git-wip-us.apache.org/repos/asf/curator/blob/f2f1953c/curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/TestPathChildrenCache.java
----------------------------------------------------------------------
diff --git a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/TestPathChildrenCache.java
b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/TestPathChildrenCache.java
index 60f2e88..70156ff 100644
--- a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/TestPathChildrenCache.java
+++ b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/TestPathChildrenCache.java
@@ -18,22 +18,44 @@
  */
 package org.apache.curator.framework.recipes.cache;
 
+import com.google.common.base.Function;
+import com.google.common.base.Joiner;
+import com.google.common.collect.Collections2;
 import com.google.common.collect.Lists;
 import org.apache.curator.framework.CuratorFramework;
 import org.apache.curator.framework.CuratorFrameworkFactory;
 import org.apache.curator.framework.api.UnhandledErrorListener;
+import org.apache.curator.framework.imps.CuratorFrameworkImpl;
 import org.apache.curator.retry.RetryOneTime;
 import org.apache.curator.test.BaseClassForTests;
 import org.apache.curator.test.KillSession;
 import org.apache.curator.test.Timing;
 import org.apache.curator.utils.CloseableUtils;
+import org.apache.log4j.Appender;
+import org.apache.log4j.AppenderSkeleton;
+import org.apache.log4j.Level;
+import org.apache.log4j.Logger;
+import org.apache.log4j.SimpleLayout;
+import org.apache.log4j.spi.LoggingEvent;
 import org.apache.zookeeper.CreateMode;
 import org.apache.zookeeper.KeeperException;
 import org.testng.Assert;
 import org.testng.annotations.Test;
+
 import java.util.Collection;
 import java.util.List;
-import java.util.concurrent.*;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.Callable;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.Exchanger;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.Semaphore;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
 
@@ -76,6 +98,67 @@ public class TestPathChildrenCache extends BaseClassForTests
     }
 
     @Test
+    public void testClientClosedDuringRefreshErrorMessage() throws Exception
+    {
+        Timing timing = new Timing();
+
+        // Fiddle with logging so we can intercept the error events for org.apache.curator
+        final List<LoggingEvent> events = Lists.newArrayList();
+        Collection<String> messages = Collections2.transform(events, new Function<LoggingEvent,
String>() {
+            @Override
+            public String apply(LoggingEvent loggingEvent) {
+                return loggingEvent.getRenderedMessage();
+            }
+        });
+        Appender appender = new AppenderSkeleton(true) {
+            @Override
+            protected void append(LoggingEvent event) {
+                if (event.getLevel().equals(Level.ERROR)) {
+                    events.add(event);
+                }
+            }
+
+            @Override
+            public void close() {
+
+            }
+
+            @Override
+            public boolean requiresLayout() {
+                return false;
+            }
+        };
+        appender.setLayout(new SimpleLayout());
+        Logger rootLogger = Logger.getLogger("org.apache.curator");
+        rootLogger.addAppender(appender);
+
+        // Check that the logging setup didn't change in a way that breaks this test
+        Logger.getLogger(CuratorFrameworkImpl.class).error("Just checking");
+        Assert.assertTrue(messages.contains("Just checking"),
+                "The test error message was not logged, this test may be broken");
+
+        // try to reproduce a bunch of times because it doesn't happen reliably
+        for (int i = 0; i < 50; i++) {
+            CuratorFramework client = CuratorFrameworkFactory.newClient(server.getConnectString(),
new RetryOneTime(1));
+            client.start();
+            try {
+                PathChildrenCache cache = new PathChildrenCache(client, "/test", true);
+                cache.start(PathChildrenCache.StartMode.BUILD_INITIAL_CACHE);
+                client.newNamespaceAwareEnsurePath("/test/aaa").ensure(client.getZookeeperClient());
+                client.setData().forPath("/test/aaa", new byte[]{1, 2, 3, 4, 5});
+                cache.rebuildNode("/test/aaa");
+                CloseableUtils.closeQuietly(cache);
+            } finally {
+                CloseableUtils.closeQuietly(client);
+            }
+        }
+
+        Assert.assertEquals(messages.size(), 1, "There should not be any error events except
for the test message, " +
+                "but got:\n" + Joiner.on("\n").join(messages));
+
+    }
+
+    @Test
     public void testAsyncInitialPopulation() throws Exception
     {
         Timing timing = new Timing();

http://git-wip-us.apache.org/repos/asf/curator/blob/f2f1953c/pom.xml
----------------------------------------------------------------------
diff --git a/pom.xml b/pom.xml
index 2129e56..635819b 100644
--- a/pom.xml
+++ b/pom.xml
@@ -279,6 +279,12 @@
             </dependency>
 
             <dependency>
+                <groupId>org.slf4j</groupId>
+                <artifactId>slf4j-log4j12</artifactId>
+                <version>1.7.6</version>
+            </dependency>
+
+            <dependency>
                 <groupId>org.mockito</groupId>
                 <artifactId>mockito-core</artifactId>
                 <version>1.9.5</version>


Mime
View raw message