Github user bdumon commented on a diff in the pull request:
https://github.com/apache/curator/pull/51#discussion_r19621734
--- Diff:
curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/PathChildrenCache.java
---
@@ -382,6 +383,11 @@ public void close() throws IOException
connectionStateListener = null;
childrenWatcher = null;
dataWatcher = null;
+
+ synchronized (backgroundTaskMonitor) {
--- End diff --
No problem, there might a problem in my reasoning, which is as follows: a
refresh might be going on when PathChildrenCache.close() is called. The refresh
runs on a background thread, the ZK event thread if I'm not mistaken. Since the
processResult will be running within a synchronized block, the
PathChildrenCache.close() method will not be able to return until the
processResult is finished, since it tries to acquire the same lock.
The second synchronized block in refresh(), around the statements
"if-closed-return, getChildren()..." is for similar reasons: the if-closed in
that block might be called before the closed state is set, while the
getChildren might be launched afterwards, but by then zookeeper might already
be closed as well. By putting it in a synchronized block, we are sure no
ZooKeeper operation is going on after returning from the close() method, and
CuratorFramework/ZooKeeper can be closed without causing errors to be logged.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
