From acd2124a7a3fc31e739428ebd1d670c1aa247f94 Mon Sep 17 00:00:00 2001 From: Nicholas Othieno Date: Tue, 21 Mar 2023 15:25:28 -0400 Subject: [PATCH] Force thread cleanup in threadpool destructor The current threadpool design expects that host/client programs that use RocksDB will explicitly call ThreadPoolImpl::Impl::JoinThreads before the threadpool destructor is called. It has been observed that a case may arise in which the join on the threads is not done and the destructor is called by the exit handler of the host program. When this happens, the threadpool destructor will hang on the call to pthread_cond_destroy as show in the truncated stack trace of a hung process below: $ pstack 3885 #0 0x000014e2288ebe63 in pthread_cond_destroy@@GLIBC_2.3.2 () from target:/lib64/libpthread.so.0 #1 0x000014e225a969af in rocksdb::ThreadPoolImpl::Impl::~Impl (this=0x14e227c472a0, __in_chrg=) at MariaDB/storage/rocksdb/rocksdb/util/threadpool_imp.cc:147 #2 std::default_delete::operator() (this=, __ptr=0x14e227c472a0) at /gcc-x.5.0/include/c++/x.5.0/bits/unique_ptr.h:78 #3 std::unique_ptr >::~unique_ptr (this=, __in_chrg=) at /gcc-x.5.0/include/c++/x.5.0/bits/unique_ptr.h:263 #4 rocksdb::ThreadPoolImpl::~ThreadPoolImpl (this=, __in_chrg=) at MariaDB/storage/rocksdb/rocksdb/util/threadpool_imp.cc:427 #5 0x000014e225af7186 in std::_Destroy (__pointer=) at /gcc-x.5.0/include/c++/x.5.0/bits/stl_construct.h:98 #6 std::_Destroy_aux::__destroy (__last=, __first=0x14e227c0f190) at /gcc-x.5.0/include/c++/x.5.0/bits/stl_construct.h:108 #7 std::_Destroy (__last=, __first=) at /gcc-x.5.0/include/c++/x.5.0/bits/stl_construct.h:137 #8 std::_Destroy (__last=0x14e227c0f1c0, __first=) at /gcc-x.5.0/include/c++/x.5.0/bits/stl_construct.h:206 #9 std::vector >::~vector (this=0x14e225f65a40 , __in_chrg=) at /gcc-x.5.0/include/c++/x.5.0/bits/stl_vector.h:434 #10 rocksdb::(anonymous namespace)::PosixEnv::~PosixEnv (this=0x14e225f65a20 , __in_chrg=) at MariaDB/storage/rocksdb/rocksdb/env/env_posix.cc:133 #11 0x000014e2285685ac in __run_exit_handlers () from target:/lib64/libc.so.6 #12 0x000014e2285685fa in exit () from target:/lib64/libc.so.6 ... The reason this hang occurs is because the threadpool condition variable may still be locked by other threads. To fix the issue, a cleanup of the threads is forced by calling join before the threadpool proceeds with the rest of it's object destruction process. In this way, we handle any corner cases were the destructor is called, but the threadpool's condition variable is still locked by another thread. Lastly in our call to ThreadPoolImpl::Impl::JoinThreads we set the boolean wait_for_jobs_to_complete = false so that the effect of destructing the threadpool is immediate. This change was tested by using MariaDB as a host program, and making it follow an execution path that directly calls the threadpool destructor as MariaDB is exiting: 1) Remove the addr2line binary on the host machine. This will make the MariaDB cleanup handler to directly call the RocksDB threadpool destructor when a MariaDB crash is instigated. 2) Instigate a crash of MariaDB by sending signal 11 to its running process `pkill -11 mysqld`. 3) Check that MariaDB is no longer running: `ps -ef | grep mysqld`. Before this change, the process would still be in existence in a hang state due to the RocksDB threadpool, but after the change the process properly exits. All new code of the whole pull request, including one or several files that are either new files or modified ones, are contributed under the BSD-new license. I am contributing on behalf of my employer Amazon Web Services, Inc. --- util/threadpool_imp.cc | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/util/threadpool_imp.cc b/util/threadpool_imp.cc index 09706cac57d..eee3c324ae7 100644 --- a/util/threadpool_imp.cc +++ b/util/threadpool_imp.cc @@ -175,7 +175,15 @@ inline ThreadPoolImpl::Impl::Impl() bgsignal_(), bgthreads_() {} -inline ThreadPoolImpl::Impl::~Impl() { assert(bgthreads_.size() == 0U); } +inline ThreadPoolImpl::Impl::~Impl() { + // In case destructor is called by host program before + // first joining the threads, force join the threads + // so that the program does not hang on shutdown due + // to the threadpool condition variable being locked + // by other RocksDB threads. + JoinThreads(false); + assert(bgthreads_.size() == 0U); +} void ThreadPoolImpl::Impl::JoinThreads(bool wait_for_jobs_to_complete) { std::unique_lock lock(mu_);