ZOOKEEPER-763. Deadlock on close w/ zkpython / c client

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

CHANGES.txt

@@ -49,6 +49,9 @@ BUGFIXES:
   ZOOKEEPER-764. Observer elected leader due to inconsistent voting view 
   (henry via mahadev)
 
+  ZOOKEEPER-763. Deadlock on close w/ zkpython / c client
+  (henry via phunt)
+
 IMPROVEMENTS:
   ZOOKEEPER-724. Improve junit test integration - log harness information 
   (phunt via mahadev)

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

@@ -1258,12 +1258,15 @@ PyObject *pyzoo_set_acl(PyObject *self, PyObject *args)
 /* Closes a connection, returns integer error code */
 PyObject *pyzoo_close(PyObject *self, PyObject *args)
 {
-  int zkhid;
+  int zkhid, ret;
   if (!PyArg_ParseTuple(args, "i", &zkhid)) {
     return NULL;
   }
   CHECK_ZHANDLE(zkhid);
-  int ret = zookeeper_close(zhandles[zkhid]);
+  zhandle_t *handle = zhandles[zkhid];
+  Py_BEGIN_ALLOW_THREADS
+  ret = zookeeper_close(handle);
+  Py_END_ALLOW_THREADS
   zhandles[zkhid] = NULL; // The zk C client frees the zhandle
   return Py_BuildValue("i", ret);
 }

src/contrib/zkpython/src/test/acl_test.py

@@ -103,7 +103,7 @@ class ACLTest(zktestbase.TestBase):
                         "/zk-python-aclverifytest",
                         "",
                         invalid_acl,
-                        zookeeper.EPHEMERAL))
+                        zookeeper.EPHEMERAL)
 
 if __name__ == '__main__':
     unittest.main()

src/contrib/zkpython/src/test/close_deadlock_test.py

@@ -0,0 +1,50 @@
+#!/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
+import time
+
+
+class CloseDeadlockTest(zktestbase.TestBase):
+  """
+  This tests for the issue found in
+  https://issues.apache.org/jira/browse/ZOOKEEPER-763
+
+  zookeeper.close blocks on waiting for all completions to
+  finish. Previously it was doing so while holding teh GIL, stopping
+  any completions from actually continuing.
+
+  This test is a failure if it does not exit within a few seconds.
+  """
+  def deadlock():
+    cv = threading.Condition()
+
+    def callback(*args):
+        cv.acquire()
+        cv.notifyAll()
+        cv.release()
+        time.sleep(1)
+
+    cv.acquire()
+    zookeeper.aget(handle, "/", None, callback)
+    cv.wait()
+    zookeeper.close(handle)
+
+
+if __name__ == '__main__':
+  unittest.main()