zkpython can segfault when statting a deleted node

git-svn-id: https://svn.apache.org/repos/asf/hadoop/zookeeper/trunk@826787 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -89,6 +89,9 @@ BUGFIXES:
   exceptions for KeeperException types (henry & pat via mahadev)
 
   ZOOKEEPER-541. zkpython limited to 256 handles (henry robinson via phunt)
+
+  ZOOKEEPER-554. zkpython can segfault when statting a deleted node
+  (henry robinson via phunt)
  
 IMPROVEMENTS:
   ZOOKEEPER-473. cleanup junit tests to eliminate false positives due to

src/contrib/zkpython/src/c/zookeeper.c

@@ -203,6 +203,9 @@ void free_pywatcher( pywatcher_t *pw)
 
 PyObject *build_stat( const struct Stat *stat )
 {
+	if (stat == NULL) { 
+		return Py_None;
+	}
   return Py_BuildValue( "{s:K, s:K, s:K, s:K,"
 			"s:i, s:i, s:i, s:K,"
 			"s:i, s:i, s:K}",

src/contrib/zkpython/src/test/async_test.py

@@ -0,0 +1,33 @@
+#!/usr/bin/python
+#
+#  Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+
+#     http://www.apache.org/licenses/LICENSE-2.0
+
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import zookeeper, zktestbase, unittest, threading
+
+class AsyncTest(zktestbase.TestBase):
+    """Test whether async works"""
+    # to do: startup and teardown via scripts?
+    def setUp( self ):
+        zktestbase.TestBase.setUp(self)
+
+    def test_async(self):
+        self.assertEqual(self.connected, True)
+        ret = zookeeper.async(self.handle, "/")
+        self.assertEqual(ret, zookeeper.OK, "async failed")
+        
+if __name__ == '__main__':
+    unittest.main()

src/contrib/zkpython/src/test/callback_test.py

@@ -0,0 +1,155 @@
+#!/usr/bin/python
+#
+#  Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+
+#     http://www.apache.org/licenses/LICENSE-2.0
+
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import zookeeper, zktestbase, unittest, threading, gc
+
+ZOO_OPEN_ACL_UNSAFE = {"perms":0x1f, "scheme":"world", "id" :"anyone"}
+
+class CallbackTest(zktestbase.TestBase):
+    """
+    Test whether callbacks (watchers/completions) are correctly invoked
+    """
+    # to do: startup and teardown via scripts?
+    def setUp(self):
+        zktestbase.TestBase.setUp(self)
+        self.cv = threading.Condition()
+
+    def create_callback(self, callback):
+        """
+        Returns a callable which signals cv and then calls callback
+        """
+        def wrapper(*args, **kwargs):
+            self.cv.acquire()
+            callback(*args, **kwargs)
+            self.cv.notify()
+            self.cv.release()
+        return wrapper
+
+    def test_none_callback(self):
+        """
+        Test that no errors are raised when None is passed as a callback.
+        """
+        self.ensureCreated("/zk-python-none-callback-test","test")
+        # To do this we need to issue two operations, waiting on the second
+        # to ensure that the first completes
+        zookeeper.get(self.handle, "/zk-python-none-callback-test", None)
+        (d,s) = zookeeper.get(self.handle, "/zk-python-none-callback-test")
+        self.assertEqual(d, "test")
+
+    def callback_harness(self, trigger, test):
+        self.callback_flag = False
+        self.cv.acquire()
+        trigger()
+        self.cv.wait(15)
+        test()
+
+    def test_dispatch_types(self):
+        """
+        Test all the various dispatch mechanisms internal to the module.
+        """
+        def dispatch_callback(*args, **kwargs):
+            self.callback_flag = True
+        self.ensureCreated("/zk-python-dispatch-test")
+        self.callback_harness( lambda: zookeeper.adelete(self.handle,
+                                                            "/zk-python-dispatch-test",
+                                                            -1,
+                                                            self.create_callback(dispatch_callback)),
+                                  lambda: self.assertEqual(True, self.callback_flag, "Void dispatch not fired"))
+
+
+        self.ensureCreated("/zk-python-dispatch-test")
+        self.callback_harness( lambda: zookeeper.aexists(self.handle,
+                                                         "/zk-python-dispatch-test",
+                                                         None,
+                                                         self.create_callback(dispatch_callback)),
+                               lambda: self.assertEqual(True, self.callback_flag, "Stat dispatch not fired"))
+
+        self.callback_harness( lambda: zookeeper.aget(self.handle,
+                                                      "/zk-python-dispatch-test",
+                                                      None,
+                                                      self.create_callback(dispatch_callback)),
+                               lambda: self.assertEqual(True, self.callback_flag, "Data dispatch not fired"))
+
+        self.callback_harness( lambda: zookeeper.aget_children(self.handle,
+                                                               "/",
+                                                               None,
+                                                               self.create_callback( dispatch_callback )),
+                               lambda: self.assertEqual(True, self.callback_flag, "Strings dispatch not fired"))
+
+        self.callback_harness( lambda: zookeeper.async(self.handle,
+                                                       "/",
+                                                       self.create_callback( dispatch_callback )),
+                               lambda: self.assertEqual(True, self.callback_flag, "String dispatch not fired"))
+
+        self.callback_harness( lambda: zookeeper.aget_acl(self.handle,
+                                                          "/",
+                                                          self.create_callback( dispatch_callback )),
+                               lambda: self.assertEqual(True, self.callback_flag, "ACL dispatch not fired"))
+
+    def test_multiple_watchers(self):
+        """
+        Test whether multiple watchers are correctly called
+        """
+        cv1, cv2 = threading.Condition(), threading.Condition()
+        def watcher1(*args, **kwargs):
+            cv1.acquire()
+            self.watcher1 = True
+            cv1.notify()
+            cv1.release()
+
+        def watcher2(*args, **kwargs):
+            cv2.acquire()
+            self.watcher2 = True
+            cv2.notify()
+            cv2.release()
+
+        nodename = "/zk-python-multiple-watcher-test"
+        self.ensureCreated(nodename, "test")
+        cv1.acquire()
+        cv2.acquire()
+        zookeeper.get(self.handle, nodename, watcher1)
+        zookeeper.get(self.handle, nodename, watcher2)
+        zookeeper.set(self.handle, nodename, "test")
+        cv1.wait(15)
+        cv2.wait(15)
+        self.assertTrue(self.watcher1 and self.watcher2, "One or more watchers failed to fire")
+
+    def test_lose_scope(self):
+        """
+        The idea is to test that the reference counting doesn't
+        fail when we retain no references outside of the module
+        """
+        self.ensureDeleted("/zk-python-lose-scope-test")
+        self.ensureCreated("/zk-python-lose-scope-test")
+        def set_watcher():
+            def fn(): self.callback_flag = True
+            self.callback_flag = False
+            zookeeper.exists(self.handle, "/zk-python-lose-scope-test",
+                             self.create_callback( lambda handle, type, state, path: fn() )
+                             )
+
+        set_watcher()
+        gc.collect()
+        self.cv.acquire()
+        zookeeper.set(self.handle, "/zk-python-lose-scope-test", "test")
+        self.cv.wait(15)
+        self.assertEqual(self.callback_flag, True)
+
+
+if __name__ == '__main__':
+    unittest.main()

src/contrib/zkpython/src/test/get_set_test.py

@@ -44,6 +44,33 @@ class GetSetTest(zktestbase.TestBase):
                           "test",
                           stat["version"]+1)
 
+    def test_stat_deleted_node(self):
+        """
+        Test for a bug that surfaced when trying to build a
+        stat object from a non-existant node.
+
+        """
+        self.ensureDeleted("/zk-python-test-deleteme")
+        self.assertRaises(zookeeper.NoNodeException,
+                          zookeeper.get,
+                          self.handle,
+                          "/zk-python-test-deleteme")
+        self.cv = threading.Condition()
+        def callback(handle, rc, value, stat):
+            self.cv.acquire()
+            self.stat = stat
+            self.rc = rc
+            self.value = value
+            self.callback_flag = True
+            self.cv.notify()
+            self.cv.release()
+        self.cv.acquire()
+        zookeeper.aget(self.handle, "/zk-python-test-deleteme", None, callback)
+        self.cv.wait(15)
+        self.assertEqual(self.callback_flag, True, "aget timed out!")
+        self.assertEqual(self.stat, None, "Stat should be none!")
+        self.assertEqual(self.value, None, "Value should be none!")
+
     def test_async_getset(self):
         self.cv = threading.Condition()
         def get_callback(handle, rc, value, stat):

src/contrib/zkpython/src/test/zktestbase.py

@@ -72,7 +72,7 @@ class TestBase(unittest.TestCase):
         return handle
 
     def ensureDeleted(self,path):
-        self.assertEqual(zookeeper.STATE_CONNECTED, zookeeper.state(self.handle), "Not connected!")
+        self.assertEqual(zookeeper.CONNECTED_STATE, zookeeper.state(self.handle), "Not connected!")
         try:
             self.assertEqual(zookeeper.OK, zookeeper.delete(self.handle, path))
         except zookeeper.NoNodeException: