From 191fd0f9e5c0cb448d53598a050b18911f47bb08 Mon Sep 17 00:00:00 2001 From: Kenneth Benzie Date: Tue, 8 Nov 2022 17:32:45 +0000 Subject: [PATCH] Fix thread leaks in the thread pool (#1553) While testing an OpenCL driver with ThreadSanitizer enabled the OpenCL-CTS suffers from thread leaks in conversions and bruteforce on posix systems. This is because `pthread_join` is never called in `ThreadPool_Exit` for the `pthread_t`s created by the thread pool. Instead, the threads are only informed to stop waiting on the condition variable which unblocks the worker thread but does not clean up after itself. ``` ThreadPool: thread 1 exiting. ThreadPool: thread 5 exiting. ThreadPool: thread 4 exiting. ThreadPool: thread 2 exiting. ThreadPool: thread 7 exiting. ThreadPool: thread 0 exiting. ThreadPool: thread 3 exiting. ThreadPool: thread 6 exiting. Thread pool exited in a orderly fashion. ================== WARNING: ThreadSanitizer: thread leak (pid=2292842) Thread T9 (tid=2292855, finished) created by main thread at: #0 pthread_create ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:969 (libtsan.so.0+0x5ad75) #1 ThreadPool_Init() (test_conversions+0x35b2c) #2 pthread_once ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:1449 (libtsan.so.0+0x4057c) #3 GetThreadCount() (test_conversions+0x36262) #4 DoTest(_cl_device_id*, Type, Type, SaturationMode, RoundingMode, _MTdata*) [clone .isra.0] (test_conversions+0x10555) #5 test_conversions(_cl_device_id*, _cl_context*, _cl_command_queue*, int) (test_conversions+0x13226) #6 callSingleTestFunction(test_definition, _cl_device_id*, int, int, unsigned long) (test_conversions+0x2e66d) #7 parseAndCallCommandLineTests(int, char const**, _cl_device_id*, int, test_definition*, int, unsigned long, int) (test_conversions+0x2fb3a) #8 runTestHarnessWithCheck(int, char const**, int, test_definition*, int, unsigned long, test_status (*)(_cl_device_id*)) (test_conversions+0x349d8) #9 main (test_conversions+0xd725) And 7 more similar thread leaks. SUMMARY: ThreadSanitizer: thread leak (OpenCL-CTS/buildbin/conversions/test_conversions+0x35b2c) in ThreadPool_Init() ``` This patch adds global state to keep track of the `pthread_t`s created by `pthread_create` in `ThreadPool_Init`. The list of `pthread_t`s is then used by `ThreadPool_Exit` to call `pthread_join` to cleanup the `pthread_t`s correctly. A near identical example, and additional explanation, can be found on [stackoverflow](https://stackoverflow.com/questions/72435574/thread-leak-detected-when-using-condition-variable-instead-of-join-with-pthrea). On the Windows path, a similar change is not necessary because `_beginthread` is used which automatically cleans up after itself when the worker thread function returns. --- test_common/harness/ThreadPool.cpp | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/test_common/harness/ThreadPool.cpp b/test_common/harness/ThreadPool.cpp index 62798045..c4abaa2e 100644 --- a/test_common/harness/ThreadPool.cpp +++ b/test_common/harness/ThreadPool.cpp @@ -23,6 +23,7 @@ // or any other POSIX system #include +#include #if defined(_WIN32) #include @@ -58,6 +59,12 @@ CRITICAL_SECTION gAtomicLock; pthread_mutex_t gAtomicLock; #endif +#if !defined(_WIN32) +// Keep track of pthread_t's created in ThreadPool_Init() so they can be joined +// in ThreadPool_Exit() and avoid thread leaks. +static std::vector pthreads; +#endif + // Atomic add operator with mem barrier. Mem barrier needed to protect state // modified by the worker functions. cl_int ThreadPool_AtomicAdd(volatile cl_int *a, cl_int b) @@ -642,6 +649,9 @@ void ThreadPool_Init(void) gThreadCount = i; break; } +#if !defined(_WIN32) + pthreads.push_back(tid); +#endif // !_WIN32 } atexit(ThreadPool_Exit); @@ -721,7 +731,20 @@ void ThreadPool_Exit(void) "still active.\n", gThreadCount.load()); else + { +#if !defined(_WIN32) + for (pthread_t pthread : pthreads) + { + if (int err = pthread_join(pthread, nullptr)) + { + log_error("Error from %d from pthread_join. Unable to join " + "work threads. ThreadPool_Exit failed.\n", + err); + } + } +#endif log_info("Thread pool exited in a orderly fashion.\n"); + } }