Skip to content

Commit 24fd606

Browse files
abhinavdangetichiyoung
authored andcommitted
Address data race identified in TOUCH command
Recent changes introduced this data race: a466ace Fix: Let go of the stored value lock (as part of queueDirty) after converting the storedValue object to an item object. 10:50:53 WARNING: ThreadSanitizer: data race (pid=111994) 10:50:53 Read of size 1 at 0x7d1000004af4 by main thread (mutexes: write M20076, read M15175): 10:50:53 #0 StoredValue::toItem(bool, unsigned short) const /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-master/ep-engine/src/stored-value.cc:754 (ep.so+0x00000011c1d7) 10:50:53 couchbase#1 EventuallyPersistentStore::getAndUpdateTtl(std::string const&, unsigned short, void const*, long) /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-master/ep-engine/src/ep.cc:2464 (ep.so+0x0000000985be) 10:50:53 couchbase#2 EventuallyPersistentEngine::touch(void const*, protocol_binary_request_header*, bool (*)(void const*, unsigned short, void const*, unsigned char, void const*, unsigned int, unsigned char, unsigned short, unsigned long, void const*)) /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-master/ep-engine/src/ep_engine.cc:4938 (ep.so+0x0000000cd151) 10:50:53 couchbase#3 processUnknownCommand(EventuallyPersistentEngine*, void const*, protocol_binary_request_header*, bool (*)(void const*, unsigned short, void const*, unsigned char, void const*, unsigned int, unsigned char, unsigned short, unsigned long, void const*)) /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-master/ep-engine/src/ep_engine.cc:1166 (ep.so+0x0000000d6864) 10:50:53 couchbase#4 EvpUnknownCommand(engine_interface*, void const*, protocol_binary_request_header*, bool (*)(void const*, unsigned short, void const*, unsigned char, void const*, unsigned int, unsigned char, unsigned short, unsigned long, void const*)) /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-master/ep-engine/src/ep_engine.cc:1365 (ep.so+0x0000000b70f8) 10:50:53 couchbase#5 mock_unknown_command(engine_interface*, void const*, protocol_binary_request_header*, bool (*)(void const*, unsigned short, void const*, unsigned char, void const*, unsigned int, unsigned char, unsigned short, unsigned long, void const*)) /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-master/memcached/programs/engine_testapp/engine_testapp.cc:382 (engine_testapp+0x0000004ce149) 10:50:53 #6 touch(engine_interface*, engine_interface_v1*, char const*, unsigned short, unsigned int) /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-master/ep-engine/tests/ep_test_apis.cc:872 (ep_testsuite.so+0x000000072284) 10:50:53 #7 test_vbucket_compact(engine_interface*, engine_interface_v1*) /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-master/ep-engine/tests/ep_testsuite.cc:1306 (ep_testsuite.so+0x00000004453c) 10:50:53 #8 execute_test(test, char const*, char const*) /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-master/memcached/programs/engine_testapp/engine_testapp.cc:1131 (engine_testapp+0x0000004cc600) 10:50:53 #9 __libc_start_main /build/buildd/eglibc-2.15/csu/libc-start.c:226 (libc.so.6+0x00000002176c) 10:50:53 10:50:53 Previous write of size 1 at 0x7d1000004af4 by thread T11 (mutexes: write M15713, write M19262): 10:50:53 #0 PersistenceCallback::callback(std::pair<int, bool>&) /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-master/ep-engine/src/stored-value.h:462 (ep.so+0x0000000adf4a) 10:50:53 couchbase#1 CouchKVStore::commitCallback(std::vector<CouchRequest*, std::allocator<CouchRequest*> >&, KVStatsCtx&, couchstore_error_t) /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-master/ep-engine/src/couch-kvstore/couch-kvstore.cc:1947 (ep.so+0x000000179431) 10:50:53 couchbase#2 _ZN12CouchKVStore17commit2couchstoreEP8CallbackIJ10KVStatsCtxEE /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-master/ep-engine/src/couch-kvstore/couch-kvstore.cc:1761 (ep.so+0x000000170e73) 10:50:53 couchbase#3 _ZN12CouchKVStore6commitEP8CallbackIJ10KVStatsCtxEE /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-master/ep-engine/src/couch-kvstore/couch-kvstore.cc:1007 (ep.so+0x000000170aa1) 10:50:53 couchbase#4 EventuallyPersistentStore::commit(unsigned short) /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-master/ep-engine/src/ep.cc:3483 (ep.so+0x00000009e916) 10:50:53 couchbase#5 EventuallyPersistentStore::flushVBucket(unsigned short) /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-master/ep-engine/src/ep.cc:3431 (ep.so+0x00000009d874) 10:50:53 #6 Flusher::flushVB() /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-master/ep-engine/src/flusher.cc:293 (ep.so+0x0000000feece) 10:50:53 #7 Flusher::step(GlobalTask*) /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-master/ep-engine/src/flusher.cc:183 (ep.so+0x0000000fd595) 10:50:53 #8 FlusherTask::run() /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-master/ep-engine/src/tasks.cc:65 (ep.so+0x000000138ae2) 10:50:53 #9 ExecutorThread::run() /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-master/ep-engine/src/executorthread.cc:115 (ep.so+0x0000000f8af6) 10:50:53 #10 launch_executor_thread(void*) /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-master/ep-engine/src/executorthread.cc:33 (ep.so+0x0000000f86a5) 10:50:53 #11 platform_thread_wrap(void*) /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-master/platform/src/cb_pthreads.cc:54 (libplatform.so.0.1.0+0x00000000551b) Change-Id: I3eaa43f4d5238d04500bb779b0a2ea785c68ed0d Reviewed-on: http://review.couchbase.org/59408 Tested-by: buildbot <build@couchbase.com> Reviewed-by: Dave Rigby <daver@couchbase.com> Reviewed-by: Chiyoung Seo <chiyoung@couchbase.com>
1 parent fe89ab1 commit 24fd606

1 file changed

Lines changed: 6 additions & 2 deletions

File tree

src/ep.cc

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2453,17 +2453,21 @@ GetValue EventuallyPersistentStore::getAndUpdateTtl(const std::string &key,
24532453
return rv;
24542454
}
24552455

2456-
if (exptime != v->getExptime()) {
2456+
const bool exptime_mutated = exptime != v->getExptime();
2457+
if (exptime_mutated) {
24572458
v->markDirty();
24582459
v->setExptime(exptime);
24592460
v->setCas(vb->nextHLCCas());
24602461
v->setRevSeqno(v->getRevSeqno()+1);
2461-
queueDirty(vb, v, &lh, NULL);
24622462
}
24632463

24642464
GetValue rv(v->toItem(v->isLocked(ep_current_time()), vbucket),
24652465
ENGINE_SUCCESS, v->getBySeqno());
24662466

2467+
if (exptime_mutated) {
2468+
queueDirty(vb, v, &lh, NULL);
2469+
}
2470+
24672471
return rv;
24682472
} else {
24692473
if (eviction_policy == VALUE_ONLY) {

0 commit comments

Comments
 (0)