MB-19247: Fix possible data race in workload.h: workloadPattern 68/62968/6
authorabhinavdangeti <abhinav@couchbase.com>
Mon, 5 Oct 2015 21:22:38 +0000 (14:22 -0700)
committerChiyoung Seo <chiyoung@couchbase.com>
Sat, 23 Apr 2016 01:02:31 +0000 (01:02 +0000)
This variable is reported in the ThreadSanitizer output as being read
by a stats function, which could result in incorrect stats printed to
the user.  However, from code inspection the compactor
(EventuallyPersistentStore::scheduleCompaction) also reads this
variable, and hence could result in incorrect task scheduling.

WARNING: ThreadSanitizer: data race (pid=24180)

  Read of size 4 at 0x7d040000f608 by main thread (mutexes: write M1308043):
    #0 WorkLoadPolicy::stringOfWorkLoadPattern() ep-engine/src/workload.h:65 (ep.so+0x0000000bee15)
    #1 EventuallyPersistentEngine::getStats(void const*, char const*, int, void (*)(char const*, unsigned short, char const*, unsigned int, void const*)) ep-engine/src/ep_engine.cc:4554 (ep.so+0x0000000c5cac)
    #2 EvpGetStats(engine_interface*, void const*, char const*, int, void (*)(char const*, unsigned short, char const*, unsigned int, void const*)) ep-engine/src/ep_engine.cc:213 (ep.so+0x0000000b4dee)
    #3 mock_get_stats(engine_interface*, void const*, char const*, int, void (*)(char const*, unsigned short, char const*, unsigned int, void const*)) memcached/programs/engine_testapp/engine_testapp.cc:239 (engine_testapp+0x0000000ba9ad)
    #4 get_int_stat(engine_interface*, engine_interface_v1*, char const*, char const*) ep-engine/tests/ep_test_apis.cc:990 (ep_testsuite.so+0x0000000aebb1)
    #5 test_access_scanner(engine_interface*, engine_interface_v1*) ep-engine/tests/ep_testsuite.cc:8569 (ep_testsuite.so+0x00000002efd7)
    #6 execute_test(test, char const*, char const*) memcached/programs/engine_testapp/engine_testapp.cc:1090 (engine_testapp+0x0000000b946c)
    #7 __libc_start_main /build/buildd/eglibc-2.19/csu/libc-start.c:287 (libc.so.6+0x000000021ec4)

  Previous write of size 4 at 0x7d040000f608 by thread T10:
    #0 WorkLoadPolicy::setWorkLoadPattern(workload_pattern_t) ep-engine/src/workload.h:76 (ep.so+0x00000013d75b)
    #1 ExecutorThread::run() ep-engine/src/executorthread.cc:112 (ep.so+0x0000000f9503)
    #2 launch_executor_thread(void*) ep-engine/src/executorthread.cc:33 (ep.so+0x0000000f9085)
    #3 platform_thread_wrap platform/src/cb_pthreads.c:23 (libplatform.so.0.1.0+0x000000003d31)

Change-Id: If1dd4885a7beefc804e425d077ff18b117be8bdd
Reviewed-on: http://review.couchbase.org/62968
Well-Formed: buildbot <build@couchbase.com>
Reviewed-by: Will Gardner <will.gardner@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
src/workload.h

index 9cca11c..bb4016c 100644 (file)
@@ -58,11 +58,11 @@ public:
     }
 
     workload_pattern_t getWorkLoadPattern(void) {
-        return workloadPattern;
+        return workloadPattern.load();
     }
 
     std::string stringOfWorkLoadPattern(void) {
-        switch (workloadPattern) {
+        switch (workloadPattern.load()) {
         case READ_HEAVY:
             return "read_heavy";
         case WRITE_HEAVY:
@@ -73,14 +73,14 @@ public:
     }
 
     void setWorkLoadPattern(workload_pattern_t pattern) {
-        workloadPattern = pattern;
+        workloadPattern.store(pattern);
     }
 
 private:
 
     int maxNumWorkers;
     int maxNumShards;
-    volatile workload_pattern_t workloadPattern;
+    AtomicValue<workload_pattern_t> workloadPattern;
 };
 
 #endif  // SRC_WORKLOAD_H_