MB-40791: Fix data race on CB3ExecutorPool::is{Hi,Low}PrioQSet

As identified by TSan when adding additional ExecutorPool tests, the two flags isLowPrioQSet & isHiPrioQSet are accessed without locks from different threads: WARNING: ThreadSanitizer: data race (pid=25602) Write of size 1 at 0x7b5000005ad8 by main thread (mutexes: write M1128287290183932680): #0 CB3ExecutorPool::_registerTaskable(Taskable&) cb3_executorpool.cc:442 (ep-engine_ep_unit_tests+0x00000048891c) #1 CB3ExecutorPool::registerTaskable(Taskable&) cb3_executorpool.cc:454 (ep-engine_ep_unit_tests+0x000000488a16) #2 ExecutorPoolTest_cancel_can_schedule_Test<TestExecutorPool>::TestBody() executorpool_test.cc:530 (ep-engine_ep_unit_tests+0x0000012e207c) ... Previous read of size 1 at 0x7b5000005ad8 by thread T38: #0 CB3ExecutorPool::getSleepQ(unsigned int) cb3_executorpool.h:112 (ep-engine_ep_unit_tests+0x000000485421) #1 CB3ExecutorPool::_nextTask(CB3ExecutorThread&, unsigned char) cb3_executorpool.cc:148 (ep-engine_ep_unit_tests+0x000000485421) #2 CB3ExecutorPool::nextTask(CB3ExecutorThread&, unsigned char) cb3_executorpool.cc:163 (ep-engine_ep_unit_tests+0x0000004854e7) #3 CB3ExecutorThread::run() cb3_executorthread.cc:129 (ep-engine_ep_unit_tests+0x00000049cf7d) #4 launch_executor_thread cb3_executorthread.cc:34 (ep-engine_ep_unit_tests+0x00000049d0ae) #5 CouchbaseThread::run() ../platform/src/cb_pthreads.cc:58 (libplatform_so.so.0.1.0+0x000000010e1b) #6 platform_thread_wrap ../platform/src/cb_pthreads.cc:71 (libplatform_so.so.0.1.0+0x000000010e1b) This appears to be a latent issue in the CB3ExecutorPool, only exposed when different priority Taskables (Buckets) are registered. Fix by making the flags atomic. Change-Id: Ibf7fda1443bad32a36914a6d1bc7b3424a3bfe75 Reviewed-on: http://review.couchbase.org/c/kv_engine/+/133889 Tested-by: Build Bot <[email protected]> Reviewed-by: James Harrison <[email protected]>
couchbase · Aug 7, 2020 · 479db94 · 479db94
1 parent 2ead98e
commit 479db94
Show file tree

Hide file tree

Showing 2 changed files with 3 additions and 3 deletions.
diff --git a/engines/ep/src/cb3_executorpool.cc b/engines/ep/src/cb3_executorpool.cc
@@ -409,7 +409,7 @@ size_t CB3ExecutorPool::schedule(ExTask task) {
 
 void CB3ExecutorPool::_registerTaskable(Taskable& taskable) {
     TaskQ* taskQ;
-    bool* whichQset;
+    std::atomic_bool* whichQset;
     const char* queueName;
     WorkLoadPolicy& workload = taskable.getWorkLoadPolicy();
     bucket_priority_t priority = workload.getBucketPriority();

diff --git a/engines/ep/src/cb3_executorpool.h b/engines/ep/src/cb3_executorpool.h
@@ -300,10 +300,10 @@ class CB3ExecutorPool : public ExecutorPool {
 
     // Global cross bucket priority queues where tasks get scheduled into ...
     TaskQ hpTaskQ; // a vector array of numTaskSets elements for high priority
-    bool isHiPrioQset;
+    std::atomic_bool isHiPrioQset;
 
     TaskQ lpTaskQ; // a vector array of numTaskSets elements for low priority
-    bool isLowPrioQset;
+    std::atomic_bool isLowPrioQset;
 
     // Numbers of registered taskables.
     size_t numTaskables;