From commits-return-7454-archive-asf-public=cust-asf.ponee.io@zookeeper.apache.org Mon Dec 10 15:29:04 2018 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx-eu-01.ponee.io (Postfix) with SMTP id F0813180648 for ; Mon, 10 Dec 2018 15:29:03 +0100 (CET) Received: (qmail 18267 invoked by uid 500); 10 Dec 2018 14:29:03 -0000 Mailing-List: contact commits-help@zookeeper.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@zookeeper.apache.org Delivered-To: mailing list commits@zookeeper.apache.org Received: (qmail 18187 invoked by uid 99); 10 Dec 2018 14:29:03 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 10 Dec 2018 14:29:03 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 01308E1295; Mon, 10 Dec 2018 14:29:03 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: andor@apache.org To: commits@zookeeper.apache.org Message-Id: <7fbd1f9f3d6a47ec915addc7cfa12d88@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: zookeeper git commit: ZOOKEEPER-1636: cleanup completion list of a failed multi request Date: Mon, 10 Dec 2018 14:29:03 +0000 (UTC) Repository: zookeeper Updated Branches: refs/heads/master b2513c114 -> b1fd480b2 ZOOKEEPER-1636: cleanup completion list of a failed multi request (from Thawan Kooburat) Author: Michael Edwards Reviewers: andor@apache.org Closes #717 from mkedwards/ZOOKEEPER-1636-for-master Project: http://git-wip-us.apache.org/repos/asf/zookeeper/repo Commit: http://git-wip-us.apache.org/repos/asf/zookeeper/commit/b1fd480b Tree: http://git-wip-us.apache.org/repos/asf/zookeeper/tree/b1fd480b Diff: http://git-wip-us.apache.org/repos/asf/zookeeper/diff/b1fd480b Branch: refs/heads/master Commit: b1fd480b2c8e0cc1429345ee04510d3849001c5c Parents: b2513c1 Author: Michael Edwards Authored: Mon Dec 10 15:28:57 2018 +0100 Committer: Andor Molnar Committed: Mon Dec 10 15:28:57 2018 +0100 ---------------------------------------------------------------------- .../zookeeper-client-c/src/zookeeper.c | 17 ++++- .../zookeeper-client-c/tests/TestMulti.cc | 68 ++++++++++++++++++++ 2 files changed, 84 insertions(+), 1 deletion(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/zookeeper/blob/b1fd480b/zookeeper-client/zookeeper-client-c/src/zookeeper.c ---------------------------------------------------------------------- diff --git a/zookeeper-client/zookeeper-client-c/src/zookeeper.c b/zookeeper-client/zookeeper-client-c/src/zookeeper.c index ab40ae9..239ffa2 100644 --- a/zookeeper-client/zookeeper-client-c/src/zookeeper.c +++ b/zookeeper-client/zookeeper-client-c/src/zookeeper.c @@ -2594,6 +2594,17 @@ completion_list_t *dequeue_completion(completion_head_t *list) return cptr; } +// cleanup completion list of a failed multi request +static void cleanup_failed_multi(zhandle_t *zh, int xid, int rc, completion_list_t *cptr) { + completion_list_t *entry; + completion_head_t *clist = &cptr->c.clist; + while ((entry = dequeue_completion(clist)) != NULL) { + // Fake failed response for all sub-requests + deserialize_response(zh, entry->c.type, xid, 1, rc, entry, NULL); + destroy_completion_entry(entry); + } +} + static int deserialize_multi(zhandle_t *zh, int xid, completion_list_t *cptr, struct iarchive *ia) { int rc = 0; @@ -2723,8 +2734,12 @@ static void deserialize_response(zhandle_t *zh, int type, int xid, int failed, i case COMPLETION_MULTI: LOG_DEBUG(LOGCALLBACK(zh), "Calling COMPLETION_MULTI for xid=%#x failed=%d rc=%d", cptr->xid, failed, rc); - rc = deserialize_multi(zh, xid, cptr, ia); assert(cptr->c.void_result); + if (failed) { + cleanup_failed_multi(zh, xid, rc, cptr); + } else { + rc = deserialize_multi(zh, xid, cptr, ia); + } cptr->c.void_result(rc, cptr->data); break; default: http://git-wip-us.apache.org/repos/asf/zookeeper/blob/b1fd480b/zookeeper-client/zookeeper-client-c/tests/TestMulti.cc ---------------------------------------------------------------------- diff --git a/zookeeper-client/zookeeper-client-c/tests/TestMulti.cc b/zookeeper-client/zookeeper-client-c/tests/TestMulti.cc index 0ee9566..ec1096d 100644 --- a/zookeeper-client/zookeeper-client-c/tests/TestMulti.cc +++ b/zookeeper-client/zookeeper-client-c/tests/TestMulti.cc @@ -178,6 +178,7 @@ class Zookeeper_multi : public CPPUNIT_NS::TestFixture CPPUNIT_TEST(testCheck); CPPUNIT_TEST(testWatch); CPPUNIT_TEST(testSequentialNodeCreateInAsyncMulti); + CPPUNIT_TEST(testBigAsyncMulti); CPPUNIT_TEST_SUITE_END(); static void watcher(zhandle_t *, int type, int state, const char *path,void*v){ @@ -248,6 +249,16 @@ public: count++; } + static void multi_completion_fn_rc(int rc, const void *data) { + count++; + *((int*) data) = rc; + } + + static void create_completion_fn_rc(int rc, const char* value, const void *data) { + count++; + *((int*) data) = rc; + } + static void waitForMultiCompletion(int seconds) { time_t expires = time(0) + seconds; while(count == 0 && time(0) < expires) { @@ -654,6 +665,63 @@ public: // wait for multi completion in doMultiInWatch waitForMultiCompletion(5); } + + /** + * ZOOKEEPER-1636: If request is too large, the server will cut the + * connection without sending response packet. The client will try to + * process completion on multi request and eventually cause SIGSEGV + */ + void testBigAsyncMulti() { + int rc; + int callback_rc = (int) ZOK; + watchctx_t ctx; + zhandle_t *zk = createClient(&ctx); + + // The request should be more than 1MB which exceeds the default + // jute.maxbuffer and causes the server to drop client connection + const int iteration = 500; + const int type_count = 3; + const int nops = iteration * type_count; + char buff[1024]; + + zoo_op_result_t results[nops]; + zoo_op_t ops[nops]; + struct Stat* s[nops]; + int index = 0; + + // Test that we deliver error to 3 types of sub-request + for (int i = 0; i < iteration; ++i) { + zoo_set_op_init(&ops[index++], "/x", buff, sizeof(buff), -1, s[i]); + zoo_create_op_init(&ops[index++], "/x", buff, sizeof(buff), + &ZOO_OPEN_ACL_UNSAFE, ZOO_SEQUENCE, NULL, 0); + zoo_delete_op_init(&ops[index++], "/x", -1); + } + + rc = zoo_amulti(zk, nops, ops, results, multi_completion_fn_rc, + + &callback_rc); + CPPUNIT_ASSERT_EQUAL((int) ZOK, rc); + + waitForMultiCompletion(10); + // With the bug, we will get SIGSEGV before reaching this point + CPPUNIT_ASSERT_EQUAL((int) ZCONNECTIONLOSS, callback_rc); + + // Make sure that all sub-request completions get processed + for (int i = 0; i < nops; ++i) { + CPPUNIT_ASSERT_EQUAL((int) ZCONNECTIONLOSS, results[i].err); + } + + // The handle should be able to recover itself. + ctx.waitForConnected(zk); + + // Try to submit another async request to see if it get processed + // correctly + rc = zoo_acreate(zk, "/target", "", 0, &ZOO_OPEN_ACL_UNSAFE, 0, + create_completion_fn_rc, &callback_rc); + CPPUNIT_ASSERT_EQUAL((int) ZOK, rc); + + waitForMultiCompletion(10); + CPPUNIT_ASSERT_EQUAL((int) ZOK, callback_rc); + } /** * ZOOKEEPER-1624: PendingChanges of create sequential node request didn't