diff --git zkc-3.4.5-orig/c/src/zookeeper.c zkc-3.4.5/c/src/zookeeper.c index de58c62..2347ff4 100644 --- zkc-3.4.5-orig/c/src/zookeeper.c +++ zkc-3.4.5/c/src/zookeeper.c @@ -1167,25 +1167,20 @@ void free_completions(zhandle_t *zh,int callCompletion,int reason) zh->outstanding_sync--; destroy_completion_entry(cptr); } else if (callCompletion) { - if(cptr->xid == PING_XID){ - // Nothing to do with a ping response - destroy_completion_entry(cptr); - } else { - // Fake the response - buffer_list_t *bptr; - h.xid = cptr->xid; - h.zxid = -1; - h.err = reason; - oa = create_buffer_oarchive(); - serialize_ReplyHeader(oa, "header", &h); - bptr = calloc(sizeof(*bptr), 1); - assert(bptr); - bptr->len = get_buffer_len(oa); - bptr->buffer = get_buffer(oa); - close_buffer_oarchive(&oa, 0); - cptr->buffer = bptr; - queue_completion(&zh->completions_to_process, cptr, 0); - } + // Fake the response + buffer_list_t *bptr; + h.xid = cptr->xid; + h.zxid = -1; + h.err = reason; + oa = create_buffer_oarchive(); + serialize_ReplyHeader(oa, "header", &h); + bptr = calloc(sizeof(*bptr), 1); + assert(bptr); + bptr->len = get_buffer_len(oa); + bptr->buffer = get_buffer(oa); + close_buffer_oarchive(&oa, 0); + cptr->buffer = bptr; + queue_completion(&zh->completions_to_process, cptr, 0); } } a_list.completion = NULL; @@ -1526,7 +1521,6 @@ static struct timeval get_timeval(int interval) rc = serialize_RequestHeader(oa, "header", &h); enter_critical(zh); gettimeofday(&zh->last_ping, 0); - rc = rc < 0 ? rc : add_void_completion(zh, h.xid, 0, 0); rc = rc < 0 ? rc : queue_buffer_bytes(&zh->to_send, get_buffer(oa), get_buffer_len(oa)); leave_critical(zh); @@ -2063,12 +2057,8 @@ static void deserialize_response(int type, int xid, int failed, int rc, completi case COMPLETION_VOID: LOG_DEBUG(("Calling COMPLETION_VOID for xid=%#x failed=%d rc=%d", cptr->xid, failed, rc)); - if (xid == PING_XID) { - // We want to skip the ping - } else { - assert(cptr->c.void_result); - cptr->c.void_result(rc, cptr->data); - } + assert(cptr->c.void_result); + cptr->c.void_result(rc, cptr->data); break; case COMPLETION_MULTI: LOG_DEBUG(("Calling COMPLETION_MULTI for xid=%#x failed=%d rc=%d", @@ -2184,7 +2174,15 @@ int zookeeper_process(zhandle_t *zh, int events) // fprintf(stderr, "Got %#x for %#x\n", hdr.zxid, hdr.xid); } - if (hdr.xid == WATCHER_EVENT_XID) { + if (hdr.xid == PING_XID) { + // Ping replies can arrive out-of-order + int elapsed = 0; + struct timeval now; + gettimeofday(&now, 0); + elapsed = calculate_interval(&zh->last_ping, &now); + LOG_DEBUG(("Got ping response in %d ms", elapsed)); + free_buffer(bptr); + } else if (hdr.xid == WATCHER_EVENT_XID) { struct WatcherEvent evt; int type = 0; char *path = NULL; @@ -2250,22 +2248,9 @@ int zookeeper_process(zhandle_t *zh, int events) activateWatcher(zh, cptr->watcher, rc); if (cptr->c.void_result != SYNCHRONOUS_MARKER) { - if(hdr.xid == PING_XID){ - int elapsed = 0; - struct timeval now; - gettimeofday(&now, 0); - elapsed = calculate_interval(&zh->last_ping, &now); - LOG_DEBUG(("Got ping response in %d ms", elapsed)); - - // Nothing to do with a ping response - free_buffer(bptr); - destroy_completion_entry(cptr); - } else { - LOG_DEBUG(("Queueing asynchronous response")); - - cptr->buffer = bptr; - queue_completion(&zh->completions_to_process, cptr, 0); - } + LOG_DEBUG(("Queueing asynchronous response")); + cptr->buffer = bptr; + queue_completion(&zh->completions_to_process, cptr, 0); } else { struct sync_completion *sc = (struct sync_completion*)cptr->data; diff --git zkc-3.4.5-orig/c/tests/TestOperations.cc zkc-3.4.5/c/tests/TestOperations.cc index b0370e9..27d9270 100644 --- zkc-3.4.5-orig/c/tests/TestOperations.cc +++ zkc-3.4.5/c/tests/TestOperations.cc @@ -29,6 +29,7 @@ class Zookeeper_operations : public CPPUNIT_NS::TestFixture CPPUNIT_TEST_SUITE(Zookeeper_operations); #ifndef THREADED CPPUNIT_TEST(testPing); + CPPUNIT_TEST(testUnsolicitedPing); CPPUNIT_TEST(testTimeoutCausedByWatches1); CPPUNIT_TEST(testTimeoutCausedByWatches2); #else @@ -305,6 +306,40 @@ public: CPPUNIT_ASSERT_EQUAL(1,zkServer.pingCount_); } + // ZOOKEEPER-2253: Permit unsolicited pings + void testUnsolicitedPing() + { + const int TIMEOUT=9; // timeout in secs + Mock_gettimeofday timeMock; + PingCountingServer zkServer; + // must call zookeeper_close() while all the mocks are in scope + CloseFinally guard(&zh); + + // receive timeout is in milliseconds + zh=zookeeper_init("localhost:1234",watcher,TIMEOUT*1000,TEST_CLIENT_ID,0,0); + CPPUNIT_ASSERT(zh!=0); + // simulate connected state + forceConnected(zh); + + int fd=0; + int interest=0; + timeval tv; + + int rc=zookeeper_interest(zh,&fd,&interest,&tv); + CPPUNIT_ASSERT_EQUAL((int)ZOK,rc); + + // verify no ping sent + CPPUNIT_ASSERT(zkServer.pingCount_==0); + + // we're going to receive a unsolicited PING response; ensure + // that the client has updated its last_recv timestamp + timeMock.tick(tv); + zkServer.addRecvResponse(new PingResponse); + rc=zookeeper_process(zh,interest); + CPPUNIT_ASSERT_EQUAL((int)ZOK,rc); + CPPUNIT_ASSERT(timeMock==zh->last_recv); + } + // simulate a watch arriving right before a ping is due // assert the ping is sent nevertheless void testTimeoutCausedByWatches1()