ZOOKEEPER-591. The C Client cannot exit properly in some situation (mahadev)

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

CHANGES.txt

@@ -293,6 +293,8 @@ BUGFIXES:
   ZOOKEEPER-624. The C Client cause core dump when receive error data from
   Zookeeper Server (mahadev)
 
+  ZOOKEEPER-591. The C Client cannot exit properly in some situation (mahadev)
+
 IMPROVEMENTS:
   ZOOKEEPER-473. cleanup junit tests to eliminate false positives due to
   "socket reuse" and failure to close client (phunt via mahadev)

src/c/src/zookeeper.c

@@ -176,9 +176,10 @@ static int add_completion(zhandle_t *zh, int xid, int completion_type,
 static completion_list_t* create_completion_entry(int xid, int completion_type,
         const void *dc, const void *data,watcher_registration_t* wo);
 static void destroy_completion_entry(completion_list_t* c);
+static void queue_completion_nolock(completion_head_t *list, completion_list_t *c,
+        int add_to_front);
 static void queue_completion(completion_head_t *list, completion_list_t *c,
         int add_to_front);
-
 static int handle_socket_error_msg(zhandle_t *zh, int line, int rc,
     const char* format,...);
 static void cleanup_bufs(zhandle_t *zh,int callCompletion,int rc);
@@ -2172,12 +2173,12 @@ static void destroy_completion_entry(completion_list_t* c){
     }
 }
 
-static void queue_completion(completion_head_t *list, completion_list_t *c,
-        int add_to_front)
+static void queue_completion_nolock(completion_head_t *list, 
+                                    completion_list_t *c,
+                                    int add_to_front) 
 {
     c->next = 0;
     /* appending a new entry to the back of the list */
-    lock_completion_list(list);
     if (list->last) {
         assert(list->head);
         // List is not empty
@@ -2194,6 +2195,14 @@ static void queue_completion(completion_head_t *list, completion_list_t *c,
         list->head = c;
         list->last = c;
     }
+}
+
+static void queue_completion(completion_head_t *list, completion_list_t *c,
+        int add_to_front)
+{
+
+    lock_completion_list(list);
+    queue_completion_nolock(list, c, add_to_front);
     unlock_completion_list(list);
 }
 
@@ -2203,9 +2212,17 @@ static int add_completion(zhandle_t *zh, int xid, int completion_type,
 {
     completion_list_t *c =create_completion_entry(xid, completion_type, dc,
             data,wo);
+    int rc = 0;
     if (!c)
         return ZSYSTEMERROR;
-    queue_completion(&zh->sent_requests, c, add_to_front);
+    lock_completion_list(&zh->sent_requests);
+    if (zh->close_requested != 1) {
+        queue_completion_nolock(&zh->sent_requests, c, add_to_front);
+        rc = ZOK;
+    } else {
+        rc = ZINVALIDSTATE;
+    }
+    unlock_completion_list(&zh->sent_requests);
     if (dc == SYNCHRONOUS_MARKER) {
         zh->outstanding_sync++;
     }
@@ -2262,6 +2279,7 @@ int zookeeper_close(zhandle_t *zh)
 
     zh->close_requested=1;
     if (inc_ref_counter(zh,0)!=0) {
+        cleanup_bufs(zh, 1, ZCLOSING);
         adaptor_finish(zh);
         return ZOK;
     }

src/c/tests/TestClient.cc

@@ -195,6 +195,7 @@ class Zookeeper_simpleSystem : public CPPUNIT_NS::TestFixture
     CPPUNIT_TEST(testAcl);
     CPPUNIT_TEST(testChroot);
     CPPUNIT_TEST(testAuth);
+    CPPUNIT_TEST(testHangingClient);
     CPPUNIT_TEST(testWatcherAutoResetWithGlobal);
     CPPUNIT_TEST(testWatcherAutoResetWithLocal);
     CPPUNIT_TEST(testGetChildren2);
@@ -277,6 +278,31 @@ public:
     void tearDown()
     {
     }
+    
+    /** have a callback in the default watcher **/
+    static void default_zoo_watcher(zhandle_t *zzh, int type, int state, const char *path, void *context){
+        int zrc = 0;
+        struct String_vector str_vec = {0, NULL};
+        zrc = zoo_wget_children(zzh, "/mytest", default_zoo_watcher, NULL, &str_vec);
+    }
+    
+    /** this checks for a deadlock in calling zookeeper_close and calls from a default watcher that might get triggered just when zookeeper_close() is in progress **/
+    void testHangingClient() {
+        int zrc = 0;
+        char buff[10] = "testall";
+        char path[512];
+        watchctx_t *ctx;
+        struct String_vector str_vec = {0, NULL};
+        zhandle_t *zh = zookeeper_init(hostPorts, NULL, 10000, 0, ctx, 0);
+        sleep(1);
+        zrc = zoo_create(zh, "/mytest", buff, 10, &ZOO_OPEN_ACL_UNSAFE, 0, path, 512);
+        zrc = zoo_wget_children(zh, "/mytest", default_zoo_watcher, NULL, &str_vec);
+        zrc = zoo_create(zh, "/mytest/test1", buff, 10, &ZOO_OPEN_ACL_UNSAFE, 0, path, 512);
+        zrc = zoo_wget_children(zh, "/mytest", default_zoo_watcher, NULL, &str_vec);
+        zrc = zoo_delete(zh, "/mytest/test1", -1);
+        zookeeper_close(zh);
+    }
+    
 
     void testPing()
     {