From 56f1cb75122f28919e50e993df85d258fc7766db Mon Sep 17 00:00:00 2001 From: "Kenneth Benzie (Benie)" Date: Fri, 28 Oct 2022 16:05:50 +0100 Subject: [PATCH] Fix thread leaks in the thread pool 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 | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/test_common/harness/ThreadPool.cpp b/test_common/harness/ThreadPool.cpp index 627980458c..0753281c39 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) @@ -634,6 +641,7 @@ void ThreadPool_Init(void) pthread_t tid = 0; err = pthread_create(&tid, NULL, ThreadPool_WorkerFunc, (void *)&threadID); + pthreads.push_back(tid); #endif // !_WIN32 if (err) { @@ -721,7 +729,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"); + } }