From daf35ce2c355fc5cdc73e0f7baf0cba5bbaeab8f Mon Sep 17 00:00:00 2001 From: Andor Molnar Date: Mon, 8 Oct 2018 16:54:29 +0200 Subject: [PATCH] ZOOKEEPER-2320. Fix invalid pointer issue on zoo_aremove_watchers() --- .../zookeeper-client-c/CMakeLists.txt | 2 +- .../zookeeper-client-c/Makefile.am | 27 ++++++++++++------- .../zookeeper-client-c/src/zk_adaptor.h | 2 +- .../src/{ => zk_hashtable}/zk_hashtable.c | 6 ++--- .../src/{ => zk_hashtable}/zk_hashtable.h | 0 .../zookeeper-client-c/src/zookeeper.c | 12 +-------- .../zookeeper-client-c/tests/TestClient.cc | 23 +++++++++++++--- 7 files changed, 43 insertions(+), 29 deletions(-) rename zookeeper-client/zookeeper-client-c/src/{ => zk_hashtable}/zk_hashtable.c (99%) rename zookeeper-client/zookeeper-client-c/src/{ => zk_hashtable}/zk_hashtable.h (100%) diff --git a/zookeeper-client/zookeeper-client-c/CMakeLists.txt b/zookeeper-client/zookeeper-client-c/CMakeLists.txt index af023e76ac8..d0246267e5f 100644 --- a/zookeeper-client/zookeeper-client-c/CMakeLists.txt +++ b/zookeeper-client/zookeeper-client-c/CMakeLists.txt @@ -162,7 +162,7 @@ set(zookeeper_sources src/recordio.c generated/zookeeper.jute.c src/zk_log.c - src/zk_hashtable.c + src/zk_hashtable/zk_hashtable.c src/addrvec.c) if(WANT_SYNCAPI) diff --git a/zookeeper-client/zookeeper-client-c/Makefile.am b/zookeeper-client/zookeeper-client-c/Makefile.am index a81e3da2200..b0308b91df7 100644 --- a/zookeeper-client/zookeeper-client-c/Makefile.am +++ b/zookeeper-client/zookeeper-client-c/Makefile.am @@ -24,14 +24,21 @@ EXTRA_DIST=LICENSE HASHTABLE_SRC = src/hashtable/hashtable_itr.h src/hashtable/hashtable_itr.c \ src/hashtable/hashtable_private.h src/hashtable/hashtable.h src/hashtable/hashtable.c -noinst_LTLIBRARIES = libhashtable.la -libhashtable_la_SOURCES = $(HASHTABLE_SRC) +ZKHASHTABLE_SRC = src/zk_hashtable/zk_hashtable.h src/zk_hashtable/zk_hashtable.c + +noinst_LTLIBRARIES = libhashtablest.la +libhashtablest_la_SOURCES = $(HASHTABLE_SRC) $(ZKHASHTABLE_SRC) + +if WANT_SYNCAPI +noinst_LTLIBRARIES += libhashtablemt.la +libhashtablemt_la_SOURCES = $(HASHTABLE_SRC) $(ZKHASHTABLE_SRC) +libhashtablemt_la_CFLAGS = -DTHREADED +endif COMMON_SRC = src/zookeeper.c include/zookeeper.h include/zookeeper_version.h include/zookeeper_log.h\ src/recordio.c include/recordio.h include/proto.h \ src/zk_adaptor.h generated/zookeeper.jute.c \ - src/zk_log.c src/zk_hashtable.h src/zk_hashtable.c \ - src/addrvec.h src/addrvec.c + src/zk_log.c src/addrvec.h src/addrvec.c # These are the symbols (classes, mostly) we want to export from our library. EXPORT_SYMBOLS = '(zoo_|zookeeper_|zhandle|Z|format_log_message|log_message|logLevel|deallocate_|allocate_|zerror|is_unrecoverable)' @@ -41,8 +48,8 @@ libzkst_la_LIBADD = -lm $(CLOCK_GETTIME_LIBS) lib_LTLIBRARIES = libzookeeper_st.la libzookeeper_st_la_SOURCES = -libzookeeper_st_la_LIBADD=libzkst.la libhashtable.la -libzookeeper_st_la_DEPENDENCIES=libzkst.la libhashtable.la +libzookeeper_st_la_LIBADD=libzkst.la libhashtablest.la +libzookeeper_st_la_DEPENDENCIES=libzkst.la libhashtablest.la libzookeeper_st_la_LDFLAGS = $(LIB_LDFLAGS) -export-symbols-regex $(EXPORT_SYMBOLS) if WANT_SYNCAPI @@ -53,8 +60,8 @@ libzkmt_la_LIBADD = -lm $(CLOCK_GETTIME_LIBS) lib_LTLIBRARIES += libzookeeper_mt.la libzookeeper_mt_la_SOURCES = -libzookeeper_mt_la_LIBADD=libzkmt.la libhashtable.la -lpthread -libzookeeper_mt_la_DEPENDENCIES=libzkmt.la libhashtable.la +libzookeeper_mt_la_LIBADD=libzkmt.la libhashtablemt.la -lpthread +libzookeeper_mt_la_DEPENDENCIES=libzkmt.la libhashtablemt.la libzookeeper_mt_la_LDFLAGS = $(LIB_LDFLAGS) -export-symbols-regex $(EXPORT_SYMBOLS) endif @@ -120,14 +127,14 @@ check_PROGRAMS = zktest-st TESTS_ENVIRONMENT = ZKROOT=${srcdir}/../.. \ CLASSPATH=$$CLASSPATH:$$CLOVER_HOME/lib/clover.jar nodist_zktest_st_SOURCES = $(TEST_SOURCES) -zktest_st_LDADD = libzkst.la libhashtable.la $(CPPUNIT_LIBS) +zktest_st_LDADD = libzkst.la libhashtablest.la $(CPPUNIT_LIBS) zktest_st_CXXFLAGS = -DUSE_STATIC_LIB $(CPPUNIT_CFLAGS) $(USEIPV6) $(SOLARIS_CPPFLAGS) zktest_st_LDFLAGS = -shared $(SYMBOL_WRAPPERS) $(SOLARIS_LIB_LDFLAGS) if WANT_SYNCAPI check_PROGRAMS += zktest-mt nodist_zktest_mt_SOURCES = $(TEST_SOURCES) tests/PthreadMocks.cc - zktest_mt_LDADD = libzkmt.la libhashtable.la -lpthread $(CPPUNIT_LIBS) + zktest_mt_LDADD = libzkmt.la libhashtablemt.la -lpthread $(CPPUNIT_LIBS) zktest_mt_CXXFLAGS = -DUSE_STATIC_LIB -DTHREADED $(CPPUNIT_CFLAGS) $(USEIPV6) if SOLARIS SHELL_SYMBOL_WRAPPERS_MT = cat ${srcdir}/tests/wrappers-mt.opt diff --git a/zookeeper-client/zookeeper-client-c/src/zk_adaptor.h b/zookeeper-client/zookeeper-client-c/src/zk_adaptor.h index 97995e36ace..bf0573c32b0 100644 --- a/zookeeper-client/zookeeper-client-c/src/zk_adaptor.h +++ b/zookeeper-client/zookeeper-client-c/src/zk_adaptor.h @@ -27,7 +27,7 @@ #endif #endif #include "zookeeper.h" -#include "zk_hashtable.h" +#include "zk_hashtable/zk_hashtable.h" #include "addrvec.h" /* predefined xid's values recognized as special by the server */ diff --git a/zookeeper-client/zookeeper-client-c/src/zk_hashtable.c b/zookeeper-client/zookeeper-client-c/src/zk_hashtable/zk_hashtable.c similarity index 99% rename from zookeeper-client/zookeeper-client-c/src/zk_hashtable.c rename to zookeeper-client/zookeeper-client-c/src/zk_hashtable/zk_hashtable.c index 9d858e27dbf..7df004dde28 100644 --- a/zookeeper-client/zookeeper-client-c/src/zk_hashtable.c +++ b/zookeeper-client/zookeeper-client-c/src/zk_hashtable/zk_hashtable.c @@ -17,9 +17,9 @@ */ #include "zk_hashtable.h" -#include "zk_adaptor.h" -#include "hashtable/hashtable.h" -#include "hashtable/hashtable_itr.h" +#include "../zk_adaptor.h" +#include "../hashtable/hashtable.h" +#include "../hashtable/hashtable_itr.h" #include #include #include diff --git a/zookeeper-client/zookeeper-client-c/src/zk_hashtable.h b/zookeeper-client/zookeeper-client-c/src/zk_hashtable/zk_hashtable.h similarity index 100% rename from zookeeper-client/zookeeper-client-c/src/zk_hashtable.h rename to zookeeper-client/zookeeper-client-c/src/zk_hashtable/zk_hashtable.h diff --git a/zookeeper-client/zookeeper-client-c/src/zookeeper.c b/zookeeper-client/zookeeper-client-c/src/zookeeper.c index ab40ae99586..a7c06d695bf 100644 --- a/zookeeper-client/zookeeper-client-c/src/zookeeper.c +++ b/zookeeper-client/zookeeper-client-c/src/zookeeper.c @@ -30,7 +30,7 @@ #include #include "zk_adaptor.h" #include "zookeeper_log.h" -#include "zk_hashtable.h" +#include "zk_hashtable/zk_hashtable.h" #include #include @@ -4045,18 +4045,8 @@ static int aremove_watches( goto done; } - if (!pathHasWatcher(zh, server_path, wtype, watcher, watcherCtx)) { - rc = ZNOWATCHER; - goto done; - } - if (local) { removeWatchers(zh, server_path, wtype, watcher, watcherCtx); -#ifdef THREADED - notify_sync_completion((struct sync_completion *)data); -#endif - rc = ZOK; - goto done; } oa = create_buffer_oarchive(); diff --git a/zookeeper-client/zookeeper-client-c/tests/TestClient.cc b/zookeeper-client/zookeeper-client-c/tests/TestClient.cc index 008bfdbf833..59d3caeb8f7 100644 --- a/zookeeper-client/zookeeper-client-c/tests/TestClient.cc +++ b/zookeeper-client/zookeeper-client-c/tests/TestClient.cc @@ -40,6 +40,7 @@ using namespace std; #include #include "Util.h" #include "ZKMocks.h" +#include "../src/zk_hashtable/zk_hashtable.h" struct buff_struct_2 { int32_t len; @@ -1373,10 +1374,10 @@ class Zookeeper_simpleSystem : public CPPUNIT_NS::TestFixture rc = zoo_set(zk, path, "nowatch", 7, -1); CPPUNIT_ASSERT(count == 0); - /* remove a specific watcher before it's added (should fail) */ + /* remove a specific watcher before it's added */ rc = zoo_remove_watches(zk, path, ZWATCHTYPE_DATA, watcher_rw, NULL, 0); - CPPUNIT_ASSERT_EQUAL((int)ZNOWATCHER, rc); + CPPUNIT_ASSERT_EQUAL((int)ZOK, rc); /* now add a specific watcher and then remove it */ rc = zoo_wget(zk, path, watcher_rw, NULL, @@ -1399,14 +1400,26 @@ class Zookeeper_simpleSystem : public CPPUNIT_NS::TestFixture watcher_rw, NULL, 0); CPPUNIT_ASSERT_EQUAL((int)ZOK, rc); + /* add a watch and remove it with local=1 */ + rc = zoo_wget(zk, "/something", watcher_rw, NULL, + buf, &blen, NULL); + CPPUNIT_ASSERT_EQUAL((int)ZOK, rc); + CPPUNIT_ASSERT_EQUAL(1, pathHasWatcher(zk, "/something", ZWATCHTYPE_ANY, watcher_rw, NULL)); + rc = zoo_remove_watches(zk, "/something", ZWATCHTYPE_DATA, + watcher_rw, NULL, 1); + CPPUNIT_ASSERT_EQUAL((int)ZOK, rc); + CPPUNIT_ASSERT_EQUAL(0, pathHasWatcher(zk, "/something", ZWATCHTYPE_ANY, watcher_rw, NULL)); + /* add a watch, stop the server, and have remove fail */ rc = zoo_wget(zk, path, watcher_rw, NULL, buf, &blen, NULL); CPPUNIT_ASSERT_EQUAL((int)ZOK, rc); stopServer(); ctx.waitForDisconnected(zk); + CPPUNIT_ASSERT_EQUAL(1, pathHasWatcher(zk, "/something", ZWATCHTYPE_ANY, watcher_rw, NULL)); rc = zoo_remove_watches(zk, path, ZWATCHTYPE_DATA, watcher_rw, NULL, 0); + CPPUNIT_ASSERT_EQUAL(1, pathHasWatcher(zk, "/something", ZWATCHTYPE_ANY, watcher_rw, NULL)); CPPUNIT_ASSERT_EQUAL((int)ZCONNECTIONLOSS, rc); /* bring the server back */ @@ -1434,9 +1447,13 @@ class Zookeeper_simpleSystem : public CPPUNIT_NS::TestFixture watcher_rw, ctx1, 1); CPPUNIT_ASSERT_EQUAL((int)ZNOWATCHER, rc); + CPPUNIT_ASSERT_EQUAL(1, pathHasWatcher(zk, "/something", ZWATCHTYPE_ANY, watcher_rw, NULL)); rc = zoo_remove_watches(zk, "/something2", ZWATCHTYPE_DATA, watcher_rw, ctx2, 1); - CPPUNIT_ASSERT_EQUAL((int)ZOK,rc); + CPPUNIT_ASSERT_EQUAL(0, pathHasWatcher(zk, "/something", ZWATCHTYPE_ANY, watcher_rw, NULL)); + CPPUNIT_ASSERT_EQUAL((int)ZCONNECTIONLOSS, rc); + + startServer(); } };