From e41c545a6daf17c80010ae3baa2bab2e8b3b730a Mon Sep 17 00:00:00 2001 From: Fu Zhe Date: Thu, 14 Apr 2022 11:58:36 +0800 Subject: [PATCH] Fix potential data race in DynamicThreadPool (#4648) close pingcap/tiflash#4595 --- dbms/src/Common/DynamicThreadPool.cpp | 34 +++++++++++++++------------ 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/dbms/src/Common/DynamicThreadPool.cpp b/dbms/src/Common/DynamicThreadPool.cpp index 482761a8bb8..5b154f8e4fd 100644 --- a/dbms/src/Common/DynamicThreadPool.cpp +++ b/dbms/src/Common/DynamicThreadPool.cpp @@ -120,26 +120,30 @@ void DynamicThreadPool::fixedWork(size_t index) void DynamicThreadPool::dynamicWork(TaskPtr initial_task) { - UPDATE_CUR_AND_MAX_METRIC(tiflash_thread_count, type_total_threads_of_thdpool, type_max_threads_of_thdpool); - executeTask(initial_task); - - DynamicNode node; - while (true) { + UPDATE_CUR_AND_MAX_METRIC(tiflash_thread_count, type_total_threads_of_thdpool, type_max_threads_of_thdpool); + executeTask(initial_task); + + DynamicNode node; + while (true) { - std::unique_lock lock(dynamic_mutex); - if (in_destructing) + { + std::unique_lock lock(dynamic_mutex); + if (in_destructing) + break; + // attach to just after head to reuse hot threads so that cold threads have chance to exit + node.appendTo(&dynamic_idle_head); + node.cv.wait_for(lock, dynamic_auto_shrink_cooldown); + node.detach(); + } + + if (!node.task) // may be timeout or cancelled break; - // attach to just after head to reuse hot threads so that cold threads have chance to exit - node.appendTo(&dynamic_idle_head); - node.cv.wait_for(lock, dynamic_auto_shrink_cooldown); - node.detach(); + executeTask(node.task); } - - if (!node.task) // may be timeout or cancelled - break; - executeTask(node.task); } + // must decrease counter after scope of `UPDATE_CUR_AND_MAX_METRIC` + // to avoid potential data race (#4595) alive_dynamic_threads.fetch_sub(1); }