MB-28635: Fix race in memcached_epoch access

Tim Bradgate · daverigby · commit 8a8d122c3315 · 2018-03-13T14:35:59.000Z
Identified in Thread Sanitizer: WARNING: ThreadSanitizer: data race (pid=28916) Read of size 8 at 0x0000019842f8 by thread T5 (mutexes: write M2756): #0 mc_time_convert_to_real_time /home/couchbase/jenkins/workspace/kv_engine-threadsanitizer-master-gcc7/kv_engine/daemon/mc_time.cc:117 (memcached+0x0000004ab4e2) couchbase#1 default_item_allocate_ex /home/couchbase/jenkins/workspace/kv_engine-threadsanitizer-master-gcc7/kv_engine/engines/default_engine/default_engine.cc:364 (default_engine.so+0x00000001d49b) couchbase#2 bucket_allocate_ex(Cookie&, DocKey const&, unsigned long, unsigned long, int, unsigned int, unsigned char, unsigned short) /home/couchbase/jenkins/workspace/kv_engine-threadsanitizer-master-gcc7/kv_engine/daemon/protocol/mcbp/engine_wrapper.cc:304 (memcached+0x000000458986) couchbase#3 MutationCommandContext::allocateNewItem() /home/couchbase/jenkins/workspace/kv_engine-threadsanitizer-master-gcc7/kv_engine/daemon/protocol/mcbp/mutation_context.cc:216 (memcached+0x0000004e02cd) couchbase#4 MutationCommandContext::step() /home/couchbase/jenkins/workspace/kv_engine-threadsanitizer-master-gcc7/kv_engine/daemon/protocol/mcbp/mutation_context.cc:57 (memcached+0x0000004e1057) couchbase#5 SteppableCommandContext::drive() /home/couchbase/jenkins/workspace/kv_engine-threadsanitizer-master-gcc7/kv_engine/daemon/protocol/mcbp/steppable_command_context.cc:33 (memcached+0x0000004f1d04) couchbase#6 add_set_replace_executor /home/couchbase/jenkins/workspace/kv_engine-threadsanitizer-master-gcc7/kv_engine/daemon/mcbp_executors.cc:169 (memcached+0x0000004b60b9) #7 add_executor /home/couchbase/jenkins/workspace/kv_engine-threadsanitizer-master-gcc7/kv_engine/daemon/mcbp_executors.cc:173 (memcached+0x0000004b60b9) #8 std::_Function_handler<void (Cookie&), void (*)(Cookie&)>::_M_invoke(std::_Any_data const&, Cookie&) /usr/local/include/c++/7.2.0/bits/std_function.h:316 (memcached+0x0000004b9cd2) #9 std::function<void (Cookie&)>::operator()(Cookie&) const /usr/local/include/c++/7.2.0/bits/std_function.h:706 (memcached+0x0000004b7c3c) #10 execute_request_packet /home/couchbase/jenkins/workspace/kv_engine-threadsanitizer-master-gcc7/kv_engine/daemon/mcbp_executors.cc:743 (memcached+0x0000004b7c3c) #11 mcbp_execute_packet(Cookie&) /home/couchbase/jenkins/workspace/kv_engine-threadsanitizer-master-gcc7/kv_engine/daemon/mcbp_executors.cc:824 (memcached+0x0000004b7c3c) #12 conn_execute /home/couchbase/jenkins/workspace/kv_engine-threadsanitizer-master-gcc7/kv_engine/daemon/statemachine_mcbp.cc:350 (memcached+0x0000004f84c7) #13 McbpStateMachine::execute() /home/couchbase/jenkins/workspace/kv_engine-threadsanitizer-master-gcc7/kv_engine/daemon/statemachine_mcbp.cc:151 (memcached+0x0000004f8c26) #14 McbpConnection::runStateMachinery() /home/couchbase/jenkins/workspace/kv_engine-threadsanitizer-master-gcc7/kv_engine/daemon/connection_mcbp.cc:792 (memcached+0x000000496b4d) #15 McbpConnection::runEventLoop(short) /home/couchbase/jenkins/workspace/kv_engine-threadsanitizer-master-gcc7/kv_engine/daemon/connection_mcbp.cc:973 (memcached+0x000000496bef) #16 run_event_loop(Connection*, short) /home/couchbase/jenkins/workspace/kv_engine-threadsanitizer-master-gcc7/kv_engine/daemon/connections.cc:159 (memcached+0x00000049b071) #17 event_handler(int, short, void*) /home/couchbase/jenkins/workspace/kv_engine-threadsanitizer-master-gcc7/kv_engine/daemon/memcached.cc:1018 (memcached+0x000000438b02) #18 event_process_active_single_queue.isra.26 <null> (libevent_core.so.2.1.8+0x00000001aa23) #19 CouchbaseThread::run() /home/couchbase/jenkins/workspace/kv_engine-threadsanitizer-master-gcc7/platform/src/cb_pthreads.cc:59 (libplatform_so.so.0.1.0+0x00000000974f) #20 platform_thread_wrap /home/couchbase/jenkins/workspace/kv_engine-threadsanitizer-master-gcc7/platform/src/cb_pthreads.cc:72 (libplatform_so.so.0.1.0+0x00000000974f) #21 <null> <null> (libtsan.so.0+0x000000024feb) Previous write of size 8 at 0x0000019842f8 by main thread: #0 mc_time_clock_tick /home/couchbase/jenkins/workspace/kv_engine-threadsanitizer-master-gcc7/kv_engine/daemon/mc_time.cc:255 (memcached+0x0000004ab9a3) couchbase#1 mc_time_clock_event_handler /home/couchbase/jenkins/workspace/kv_engine-threadsanitizer-master-gcc7/kv_engine/daemon/mc_time.cc:210 (memcached+0x0000004abbde) couchbase#2 event_process_active_single_queue.isra.26 <null> (libevent_core.so.2.1.8+0x00000001aa8a) couchbase#3 main /home/couchbase/jenkins/workspace/kv_engine-threadsanitizer-master-gcc7/kv_engine/daemon/main.cc:33 (memcached+0x00000041080e) Location is global 'memcached_epoch' of size 8 at 0x0000019842f8 (memcached+0x0000019842f8) Mutex M2756 (0x7b74000005c0) created at: #0 pthread_mutex_lock <null> (libtsan.so.0+0x00000003876f) couchbase#1 __gthread_mutex_lock /usr/local/include/c++/7.2.0/x86_64-pc-linux-gnu/bits/gthr-default.h:748 (memcached+0x00000046d413) couchbase#2 std::mutex::lock() /usr/local/include/c++/7.2.0/bits/std_mutex.h:103 (memcached+0x00000046d413) couchbase#3 std::lock_guard<std::mutex>::lock_guard(std::mutex&) /usr/local/include/c++/7.2.0/bits/std_mutex.h:162 (memcached+0x00000046d413) couchbase#4 thread_libevent_process /home/couchbase/jenkins/workspace/kv_engine-threadsanitizer-master-gcc7/kv_engine/daemon/thread.cc:293 (memcached+0x00000046d413) couchbase#5 event_process_active_single_queue.isra.26 <null> (libevent_core.so.2.1.8+0x00000001aa23) couchbase#6 CouchbaseThread::run() /home/couchbase/jenkins/workspace/kv_engine-threadsanitizer-master-gcc7/platform/src/cb_pthreads.cc:59 (libplatform_so.so.0.1.0+0x00000000974f) #7 platform_thread_wrap /home/couchbase/jenkins/workspace/kv_engine-threadsanitizer-master-gcc7/platform/src/cb_pthreads.cc:72 (libplatform_so.so.0.1.0+0x00000000974f) #8 <null> <null> (libtsan.so.0+0x000000024feb) Thread T5 'mc:worker_2' (tid=28923, running) created by main thread at: #0 pthread_create <null> (libtsan.so.0+0x0000000282a0) couchbase#1 cb_create_named_thread /home/couchbase/jenkins/workspace/kv_engine-threadsanitizer-master-gcc7/platform/src/cb_pthreads.cc:110 (libplatform_so.so.0.1.0+0x000000009447) couchbase#2 create_worker /home/couchbase/jenkins/workspace/kv_engine-threadsanitizer-master-gcc7/kv_engine/daemon/thread.cc:86 (memcached+0x00000046df49) couchbase#3 thread_init(unsigned long, event_base*, void (*)(int, short, void*)) /home/couchbase/jenkins/workspace/kv_engine-threadsanitizer-master-gcc7/kv_engine/daemon/thread.cc:515 (memcached+0x00000046df49) couchbase#4 memcached_main /home/couchbase/jenkins/workspace/kv_engine-threadsanitizer-master-gcc7/kv_engine/daemon/memcached.cc:2548 (memcached+0x00000043de8d) couchbase#5 main /home/couchbase/jenkins/workspace/kv_engine-threadsanitizer-master-gcc7/kv_engine/daemon/main.cc:33 (memcached+0x00000041080e) SUMMARY: ThreadSanitizer: data race /home/couchbase/jenkins/workspace/kv_engine-threadsanitizer-master-gcc7/kv_engine/daemon/mc_time.cc:117 in mc_time_convert_to_real_time Change-Id: I70c3ef30cdf4f9282505f797772c5684c8cfab9d Reviewed-on: http://review.couchbase.org/90802 Tested-by: Build Bot <build@couchbase.com> Reviewed-by: Dave Rigby <daver@couchbase.com>
diff --git a/daemon/mc_time.cc b/daemon/mc_time.cc
@@ -51,7 +51,7 @@ const time_t memcached_check_system_time = 60;
 const time_t memcached_maximum_relative_time = 60*60*24*30;
 
 static std::atomic<rel_time_t> memcached_uptime(0);
-static volatile time_t memcached_epoch = 0;
+static std::atomic<time_t> memcached_epoch(0);
 static volatile uint64_t memcached_monotonic_start = 0;
 static struct event_base* main_ev_base = NULL;
 
@@ -114,7 +114,7 @@ static_assert(std::is_unsigned<rel_time_t>::value,
 rel_time_t mc_time_convert_to_real_time(rel_time_t t, cb::ExpiryLimit limit) {
     rel_time_t rv = 0;
 
-    int64_t epoch{memcached_epoch};
+    int64_t epoch{memcached_epoch.load()};
     int64_t uptime{memcached_uptime.load()};
 
     if (t > memcached_maximum_relative_time) { // t is absolute
@@ -245,14 +245,14 @@ void mc_time_clock_tick(void) {
                         "memcached_uptime = {}, new memcached_epoch = {}, "
                         "next check {}",
                         difference,
-                        memcached_epoch,
+                        memcached_epoch.load(),
                         memcached_uptime.load(),
                         (timeofday.tv_sec - memcached_uptime),
                         check_system_time + memcached_check_system_time);
             }
             /* adjust memcached_epoch to ensure correct timeofday can
                be calculated by clients*/
-            memcached_epoch = timeofday.tv_sec - memcached_uptime;
+            memcached_epoch.store(timeofday.tv_sec - memcached_uptime);
         }
 
         /* move our checksystem time marker to trigger the next check
