MB-19380: Address data race observed with vb's pendingBGFetches

abhinavdangeti · chiyoung · commit 61c4b2d54904 · 2016-04-27T23:51:58.000Z
[Not a backport, this code was altered/removed in master] 11:56:19 Read of size 8 at 0x7d6400050df8 by main thread (mutexes: write M45364, write M44294): 11:56:19 #0 std::_Hashtable<std::string, std::pair<std::string const, std::list<VBucketBGFetchItem*, std::allocator<VBucketBGFetchItem*> > >, std::allocator<std::pair<std::string const, std::list<VBucketBGFetchItem*, std::allocator<VBucketBGFetchItem*> > > >, std::__detail::_Select1st, std::equal_to<std::string>, std::hash<std::string>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<true, false, true> >::size() const /usr/bin/../lib/gcc/x86_64-linux-gnu/4.9/../../../../include/c++/4.9/bits/hashtable.h:500 (ep.so+0x00000008d98e) 11:56:19 #1 std::unordered_map<std::string, std::list<VBucketBGFetchItem*, std::allocator<VBucketBGFetchItem*> >, std::hash<std::string>, std::equal_to<std::string>, std::allocator<std::pair<std::string const, std::list<VBucketBGFetchItem*, std::allocator<VBucketBGFetchItem*> > > > >::size() const /usr/bin/../lib/gcc/x86_64-linux-gnu/4.9/../../../../include/c++/4.9/bits/unordered_map.h:264 (ep.so+0x000000085be0) 11:56:19 #2 VBucket::numPendingBGFetchItems() /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-3.0.x/ep-engine/src/vbucket.h:333 (ep.so+0x000000101789) 11:56:19 #3 EventuallyPersistentStore::bgFetch(std::string const&, unsigned short, unsigned long, void const*, bool) /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-3.0.x/ep-engine/src/ep.cc:1651 (ep.so+0x0000000d71f7) 11:56:19 #4 EventuallyPersistentStore::getInternal(std::string const&, unsigned short, void const*, bool, bool, vbucket_state_t, bool) /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-3.0.x/ep-engine/src/ep.cc:1708 (ep.so+0x0000000e3e21) 11:56:19 #5 EventuallyPersistentStore::get(std::string const&, unsigned short, void const*, bool, bool, bool) /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-3.0.x/ep-engine/src/ep.h:242 (ep.so+0x00000019eee9) 11:56:19 #6 EventuallyPersistentEngine::get(void const*, void**, void const*, int, unsigned short, bool) /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-3.0.x/ep-engine/src/ep_engine.h:259 (ep.so+0x00000016fe75) 11:56:19 #7 EvpGet(engine_interface*, void const*, void**, void const*, int, unsigned short) /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-3.0.x/ep-engine/src/ep_engine.cc:202 (ep.so+0x000000136911) 11:56:19 #8 mock_get /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-3.0.x/memcached/programs/engine_testapp/engine_testapp.c (engine_testapp+0x0000004c6e84) 11:56:19 #9 get_item_info(engine_interface*, engine_interface_v1*, item_info*, char const*, unsigned short) /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-3.0.x/ep-engine/tests/ep_test_apis.cc:356 (ep_testsuite.so+0x0000000e2f2d) 11:56:19 #10 check_key_value(engine_interface*, engine_interface_v1*, char const*, char const*, unsigned long, unsigned short) /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-3.0.x/ep-engine/tests/ep_testsuite.cc:155 (ep_testsuite.so+0x0000000b2590) 11:56:19 #11 test_duplicate_items_disk(engine_interface*, engine_interface_v1*) /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-3.0.x/ep-engine/tests/ep_testsuite.cc:7839 (ep_testsuite.so+0x00000005a4b6) 11:56:19 #12 execute_test /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-3.0.x/memcached/programs/engine_testapp/engine_testapp.c (engine_testapp+0x0000004c4e9f) 11:56:19 #13 main crtstuff.c (engine_testapp+0x0000004c2e01) 11:56:19 11:56:19 Previous write of size 8 at 0x7d6400050df8 by thread T1 (mutexes: write M44318): 11:56:19 #0 std::_Hashtable<std::string, std::pair<std::string const, std::list<VBucketBGFetchItem*, std::allocator<VBucketBGFetchItem*> > >, std::allocator<std::pair<std::string const, std::list<VBucketBGFetchItem*, std::allocator<VBucketBGFetchItem*> > > >, std::__detail::_Select1st, std::equal_to<std::string>, std::hash<std::string>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<true, false, true> >::clear() /usr/bin/../lib/gcc/x86_64-linux-gnu/4.9/../../../../include/c++/4.9/bits/hashtable.h:1943 (ep.so+0x000000087ba8) 11:56:19 #1 std::unordered_map<std::string, std::list<VBucketBGFetchItem*, std::allocator<VBucketBGFetchItem*> >, std::hash<std::string>, std::equal_to<std::string>, std::allocator<std::pair<std::string const, std::list<VBucketBGFetchItem*, std::allocator<VBucketBGFetchItem*> > > > >::clear() /usr/bin/../lib/gcc/x86_64-linux-gnu/4.9/../../../../include/c++/4.9/bits/unordered_map.h:528 (ep.so+0x000000086f40) 11:56:19 #2 VBucket::getBGFetchItems(std::unordered_map<std::string, std::list<VBucketBGFetchItem*, std::allocator<VBucketBGFetchItem*> >, std::hash<std::string>, std::equal_to<std::string>, std::allocator<std::pair<std::string const, std::list<VBucketBGFetchItem*, std::allocator<VBucketBGFetchItem*> > > > >&) /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-3.0.x/ep-engine/src/vbucket.cc:294 (ep.so+0x0000002b4026) 11:56:19 #3 BgFetcher::run(GlobalTask*) /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-3.0.x/ep-engine/src/bgfetcher.cc:155 (ep.so+0x00000008449f) 11:56:19 #4 BgFetcherTask::run() /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-3.0.x/ep-engine/src/tasks.cc:89 (ep.so+0x00000025205e) 11:56:19 #5 ExecutorThread::run() /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-3.0.x/ep-engine/src/executorthread.cc:109 (ep.so+0x0000001e38f1) 11:56:19 #6 launch_executor_thread(void*) /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-3.0.x/ep-engine/src/executorthread.cc:34 (ep.so+0x0000001e2f1a) 11:56:19 #7 platform_thread_wrap /home/couchbase/jenkins/workspace/ep-engine-threadsanitizer-3.0.x/platform/src/cb_pthreads.c (libplatform.so.0.1.0+0x00000000377c) Change-Id: I66e3c2af1f58448a68fbfedf3dfa030a657ed9a7 Reviewed-on: http://review.couchbase.org/63369 Well-Formed: buildbot <build@couchbase.com> Tested-by: buildbot <build@couchbase.com> Reviewed-by: Will Gardner <will.gardner@couchbase.com> Reviewed-by: Chiyoung Seo <chiyoung@couchbase.com>
diff --git a/src/vbucket.h b/src/vbucket.h
@@ -329,6 +329,7 @@ class VBucket : public RCValue {
     void queueBGFetchItem(const std::string &key, VBucketBGFetchItem *fetch,
                           BgFetcher *bgFetcher);
     size_t numPendingBGFetchItems(void) {
+        LockHolder lh(pendingBGFetchesLock);
         // do a dirty read of number of fetch items
         return pendingBGFetches.size();
     }

Original file line number	Diff line number	Diff line change
`@@ -329,6 +329,7 @@ class VBucket : public RCValue {`
`329`	`329`	`void queueBGFetchItem(const std::string &key, VBucketBGFetchItem *fetch,`
`330`	`330`	`BgFetcher *bgFetcher);`
`331`	`331`	`size_t numPendingBGFetchItems(void) {`
	`332`	`+ LockHolder lh(pendingBGFetchesLock);`
`332`	`333`	`// do a dirty read of number of fetch items`
`333`	`334`	`return pendingBGFetches.size();`
`334`	`335`	`}`