Github user anmolnar commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/529#discussion_r196712402
--- Diff: src/java/test/org/apache/zookeeper/test/WatcherTest.java ---
@@ -140,6 +142,55 @@ public void processResult(int rc, String path, Object
ctx) {
}
}
}
+
+ @Test
+ public void testWatcherDisconnectOnClose()
+ throws IOException, InterruptedException, KeeperException
+ {
+ ZooKeeper zk = null;
+ try {
+ final BlockingQueue<WatchedEvent> queue = new
LinkedBlockingQueue<>();
+
+ MyWatcher connWatcher = new MyWatcher();
+
+ Watcher watcher = new Watcher(){
+ @Override
+ public void process(WatchedEvent event) {
+ try {
+ queue.put(event);
+ } catch (InterruptedException e) {
+ // Oh well, never mind
+ }
+ }
+
+ };
+
+ zk = createClient(connWatcher, hostPort);
+
+ StatCallback scb = new StatCallback() {
+ public void processResult(int rc, String path, Object ctx,
+ Stat stat) {
+ // don't do anything
+ }
+ };
+
+ // Register a watch on the node
+ zk.exists("/missing", watcher, scb, null);
+
+ // Close the client without changing the node
+ zk.close();
+
+
+ WatchedEvent event = queue.poll(10, TimeUnit.SECONDS);
+ Assert.assertEquals(Event.EventType.None, event.getType());
--- End diff --
When your patch isn't applied, the watcher won't receive any event and
timeout occurs. Resulting in `event == null` and test will get
NullReferenceException. Would you please assert for null ref and also add some
meaningful messages to the assert statements which indicate what exactly went
wrong and what should be the expected behaviour.
---
