From 9c59f89a7fc736cd26a2d271ac56afd37c42237b Mon Sep 17 00:00:00 2001 From: Michael Wallner Date: Fri, 30 Oct 2020 16:02:34 +0100 Subject: [PATCH] memaslap: fix leaks --- .cirrus.yml | 5 ++++- CMake/CheckDependency.cmake | 2 +- CMake/_Include.cmake | 1 - src/bin/CMakeLists.txt | 10 ++++++++-- src/bin/memaslap/ms_conn.c | 2 +- src/bin/memaslap/ms_thread.c | 16 ++++++++++------ src/bin/memaslap/ms_thread.h | 1 + src/libmemcached/CMakeLists.txt | 7 ++++++- test/tests/bin/memaslap.cpp | 2 +- 9 files changed, 32 insertions(+), 14 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index 43098905..adc3b3c2 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -42,7 +42,10 @@ task: svn co https://svn.freebsd.org/ports/tags/${ports}/Templates /usr/ports/Templates cd memcached echo bin/memcached > pkg-plist - make all install SASLPWDB_CONFIGURE_ENABLE=sasl-pwdb OPTIONS_SET="SASL SASLPWDB" OPTIONS_DEFINE="SASL SASLPWDB" + make all install \ + SASLPWDB_CONFIGURE_ENABLE=sasl-pwdb \ + OPTIONS_SET="SASL SASLPWDB" \ + OPTIONS_DEFINE="SASL SASLPWDB" cd .. prepare_script: | mkdir build diff --git a/CMake/CheckDependency.cmake b/CMake/CheckDependency.cmake index 862321fb..bc7ecaa2 100644 --- a/CMake/CheckDependency.cmake +++ b/CMake/CheckDependency.cmake @@ -35,7 +35,7 @@ endfunction() function(check_dependency NAME LIB HEADER) if(PKG_CONFIG_FOUND) - pkg_check_modules(${NAME} lib${LIB}${ARGN}) + pkg_check_modules(${NAME} lib${LIB}${ARGN} IMPORTED_TARGET) if(NOT ${NAME}_FOUND) pkg_check_modules(${NAME} ${LIB}${ARGN}) endif() diff --git a/CMake/_Include.cmake b/CMake/_Include.cmake index 8a33f964..c9af10ba 100644 --- a/CMake/_Include.cmake +++ b/CMake/_Include.cmake @@ -61,7 +61,6 @@ if(BUILD_TESTING) find_package(Memcached) set(MEMCACHED_BINARY ${MEMCACHED_EXECUTABLE}) endif() - check_dependency(LIBUUID uuid uuid/uuid.h) endif() ## sasl diff --git a/src/bin/CMakeLists.txt b/src/bin/CMakeLists.txt index a8d79732..c6e11b4d 100644 --- a/src/bin/CMakeLists.txt +++ b/src/bin/CMakeLists.txt @@ -32,8 +32,14 @@ if(HAVE_MEMASLAP) memaslap/ms_stats.c memaslap/ms_task.c memaslap/ms_thread.c) - target_include_directories(memaslap PRIVATE memaslap ${LIBEVENT_INCLUDEDIR}) - target_link_libraries(memaslap PRIVATE libclient_common ${LIBEVENT_LIBRARIES} Threads::Threads) + target_include_directories(memaslap PRIVATE memaslap) + target_link_libraries(memaslap PRIVATE libclient_common Threads::Threads) + if(PKG_CONFIG_FOUND) + target_link_libraries(memaslap PRIVATE PkgConfig::LIBEVENT) + else() + target_include_directories(memaslap PRIVATE ${LIBEVENT_INCLUDEDIR}) + target_link_libraries(memaslap PRIVATE ${LIBEVENT_LIBRARIES}) + endif() if(CMAKE_INSTALL_RPATH) set_target_properties(${CLIENT} PROPERTIES INSTALL_RPATH ${CMAKE_INSTALL_RPATH}/../${CMAKE_INSTALL_LIBDIR}) diff --git a/src/bin/memaslap/ms_conn.c b/src/bin/memaslap/ms_conn.c index 49b50121..f4375ae8 100644 --- a/src/bin/memaslap/ms_conn.c +++ b/src/bin/memaslap/ms_conn.c @@ -605,7 +605,7 @@ static void ms_conn_close(ms_conn_t *c) { } if (ms_thread->nactive_conn == 0) { - pthread_exit(NULL); + event_base_loopbreak(ms_thread->base); } } /* ms_conn_close */ diff --git a/src/bin/memaslap/ms_thread.c b/src/bin/memaslap/ms_thread.c index cfd46de3..1d960cfa 100644 --- a/src/bin/memaslap/ms_thread.c +++ b/src/bin/memaslap/ms_thread.c @@ -40,7 +40,7 @@ static void ms_clock_handler(const int fd, const short which, void *arg); static uint32_t ms_set_thread_cpu_affinity(uint32_t cpu); static int ms_setup_thread(ms_thread_ctx_t *thread_ctx); static void *ms_worker_libevent(void *arg); -static void ms_create_worker(void *(*func)(void *), void *arg); +static void ms_create_worker(void *(*func)(void *), ms_thread_ctx_t *arg); /** * time-sensitive callers can call it by hand with this, @@ -167,7 +167,7 @@ static int ms_setup_thread(ms_thread_ctx_t *thread_ctx) { gettimeofday(&ms_thread->startup_time, NULL); - ms_thread->base = event_init(); + ms_thread->base = event_base_new(); if (ms_thread->base == NULL) { if (atomic_add_32_nv(&cnt, 1) == 0) { fprintf(stderr, "Can't allocate event base.\n"); @@ -234,6 +234,8 @@ static void *ms_worker_libevent(void *arg) { ms_thread = pthread_getspecific(ms_thread_key); event_base_loop(ms_thread->base, 0); + event_base_free(ms_thread->base); + free(ms_thread); return NULL; } /* ms_worker_libevent */ @@ -244,14 +246,13 @@ static void *ms_worker_libevent(void *arg) { * @param func, the callback function * @param arg, the argument to pass to the callback function */ -static void ms_create_worker(void *(*func)(void *), void *arg) { - pthread_t thread; +static void ms_create_worker(void *(*func)(void *), ms_thread_ctx_t *arg) { pthread_attr_t attr; int ret; pthread_attr_init(&attr); - if ((ret = pthread_create(&thread, &attr, func, arg))) { + if ((ret = pthread_create(&arg->pth_id, &attr, func, arg))) { fprintf(stderr, "Can't create thread: %s.\n", strerror(ret)); exit(1); } @@ -286,12 +287,15 @@ void ms_thread_init() { } /* Create threads after we've done all the epoll setup. */ for (uint32_t i = 0; i < ms_setting.nthreads; i++) { - ms_create_worker(ms_worker_libevent, (void *) &ms_thread_ctx[i]); + ms_create_worker(ms_worker_libevent, &ms_thread_ctx[i]); } } /* ms_thread_init */ /* cleanup some resource of threads when all the threads exit */ void ms_thread_cleanup() { + for (uint32_t i = 0; i < ms_setting.nthreads; i++) { + pthread_join(ms_thread_ctx[i].pth_id, NULL); + } if (ms_thread_ctx) { free(ms_thread_ctx); } diff --git a/src/bin/memaslap/ms_thread.h b/src/bin/memaslap/ms_thread.h index 6ae1bb7b..eff2e557 100644 --- a/src/bin/memaslap/ms_thread.h +++ b/src/bin/memaslap/ms_thread.h @@ -33,6 +33,7 @@ typedef struct thread_ctx { uint32_t srv_idx; /* index of the thread */ int tps_perconn; /* expected throughput per connection */ int64_t exec_num_perconn; /* execute number per connection */ + pthread_t pth_id; } ms_thread_ctx_t; /* Used to store the private variables of each thread */ diff --git a/src/libmemcached/CMakeLists.txt b/src/libmemcached/CMakeLists.txt index ff940fae..d51295d5 100644 --- a/src/libmemcached/CMakeLists.txt +++ b/src/libmemcached/CMakeLists.txt @@ -91,12 +91,17 @@ set_target_properties(libmemcached PROPERTIES VERSION v${LIBMEMCACHED_VERSION}) target_compile_definitions(libmemcached PRIVATE -DBUILDING_LIBMEMCACHED) target_link_libraries(libmemcached PUBLIC libhashkit Threads::Threads ${LIBSASL_LIBRARIES} ${CMAKE_DL_LIBS} ${Backtrace_LIBRARIES}) +if(PKG_CONFIG_FOUND) + target_link_libraries(libmemcached PUBLIC libhashkit PkgConfig::LIBSASL) +else() + target_link_libraries(libmemcached PUBLIC ${LIBSASL_LIBRARIES}) + target_include_directories(libmemcached PUBLIC ${LIBSASL_INCLUDEDIR}) +endif() target_include_directories(libmemcached PRIVATE ${Backtrace_INCLUDE_DIR}) target_include_directories(libmemcached PRIVATE ${CMAKE_SOURCE_DIR}/src ${CMAKE_BINARY_DIR}/src ${CMAKE_BINARY_DIR}) -target_include_directories(libmemcached PUBLIC ${LIBSASL_INCLUDEDIR}) target_include_directories(libmemcached PUBLIC $ $ diff --git a/test/tests/bin/memaslap.cpp b/test/tests/bin/memaslap.cpp index ccac5994..65d58374 100644 --- a/test/tests/bin/memaslap.cpp +++ b/test/tests/bin/memaslap.cpp @@ -37,7 +37,7 @@ TEST_CASE("bin/memaslap") { " -T 2 -c 8 -t 2s -v 0.2 -e 0.05 -b", " -T 2 -c 8 -t 2s -w 40k -o 0.2", " -T 2 -c 8 -t 2s -d 20 -P 40k", -#if __amd64__ +#if 0 " -T 2 -c 8 -t 2s -d 50 -a -n 10", #endif }; -- 2.30.2