MB-19359: [2] Address lock inversion with vb's state lock and snapshot lock 66/63366/6
authorabhinavdangeti <abhinav@couchbase.com>
Tue, 26 Apr 2016 18:03:18 +0000 (11:03 -0700)
committerChiyoung Seo <chiyoung@couchbase.com>
Wed, 27 Apr 2016 23:37:39 +0000 (23:37 +0000)
+ [Not a backport, this code was altered/removed in master]
+ Address this lock inversion by moving reading the vbucket snapshot
  range to outside the vbucket's state lock context.

WARNING: ThreadSanitizer: lock-order-inversion (potential deadlock) (pid=245522)
  Cycle in lock order graph: M21518 (0x7d640003e220) => M21515 (0x7d640003e0f0) => M21518

  Mutex M21515 acquired here while holding mutex M21518 in thread T17:
    #0 pthread_rwlock_rdlock <null> (engine_testapp+0x000000462260)
    #1 cb_rw_reader_enter <null> (libplatform.so.0.1.0+0x000000004800)
    #2 RWLock::readerLock() ep-engine/src/rwlock.h:38 (ep.so+0x000000132360)
    #3 ReaderLockHolder::ReaderLockHolder(RWLock&) ep-engine/src/locks.h:167 (ep.so+0x0000000f8087)
    #4 EventuallyPersistentStore::addTAPBackfillItem(Item const&, unsigned char, bool) ep-engine/src/ep.cc:851 (ep.so+0x0000000d9ba7)
    #5 PassiveStream::commitMutation(MutationResponse*, bool) ep-engine/src/dcp-stream.cc:1370 (ep.so+0x00000029dd8c)
    #6 PassiveStream::processMutation(MutationResponse*) ep-engine/src/dcp-stream.cc:1346 (ep.so+0x00000029cbd0)
    #7 PassiveStream::processBufferedMessages(unsigned int&) ep-engine/src/dcp-stream.cc:1286 (ep.so+0x00000029c522)
    #8 DccpConsumer::processBufferedItems() ep-engine/src/dcp-consumer.cc:599 (ep.so+0x000000262e04)
    #9 Processer::run() ep-engine/src/dcp-consumer.cc:48 (ep.so+0x0000002629ff)
    #10 ExecutorThread::run() ep-engine/src/executorthread.cc:109 (ep.so+0x0000001e38f1)
    #11 launch_executor_thread(void*) ep-engine/src/executorthread.cc:34 (ep.so+0x0000001e2f1a)
    #12 platform_thread_wrap platform/src/cb_pthreads.c (libplatform.so.0.1.0+0x00000000377c)

  Mutex M21518 acquired here while holding mutex M21515 in main thread:
    #0 pthread_mutex_lock <null> (engine_testapp+0x00000047e9e0)
    #1 cb_mutex_enter <null> (libplatform.so.0.1.0+0x0000000039c0)
    #2 Mutex::acquire() ep-engine/src/mutex.cc:31 (ep.so+0x0000001e241e)
    #3 LockHolder::lock() ep-engine/src/locks.h:71 (ep.so+0x000000080bc3)
    #4 LockHolder::LockHolder(Mutex&, bool) ep-engine/src/locks.h:48 (ep.so+0x000000080832)
    #5 VBucket::getCurrentSnapshot(unsigned long&, unsigned long&) ep-engine/src/vbucket.h:233 (ep.so+0x0000000fae05)
    #6 EventuallyPersistentEngine::doSeqnoStats(void const*, void (*)(char const*, unsigned short, char const*, unsigned int, void const*), char const*, int) ep-engine/src/ep_engine.cc:4255 (ep.so+0x00000014f202)
    #7 EventuallyPersistentEngine::getStats(void const*, char const*, int, void (*)(char const*, unsigned short, char const*, unsigned int, void const*)) ep-engine/src/ep_engine.cc:4372 (ep.so+0x000000150bcb)
    #8 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:214 (ep.so+0x000000136a72)
    #9 mock_get_stats memcached/programs/engine_testapp/engine_testapp.c (engine_testapp+0x0000004c8403)
    #10 get_int_stat(engine_interface*, engine_interface_v1*, char const*, char const*) ep-engine/tests/ep_test_apis.cc:771 (ep_testsuite.so+0x0000000e21ea)
    #11 wait_for_stat_to_be(engine_interface*, engine_interface_v1*, char const*, int, char const*) ep-engine/tests/ep_test_apis.cc:860 (ep_testsuite.so+0x0000000e8d2b)
    #12 test_dcp_replica_stream_backfill(engine_interface*, engine_interface_v1*) ep-engine/tests/ep_testsuite.cc:5306 (ep_testsuite.so+0x00000008e601)
    #13 execute_test memcached/programs/engine_testapp/engine_testapp.c (engine_testapp+0x0000004c4e9f)
    #14 main crtstuff.c (engine_testapp+0x0000004c2e01)

Change-Id: Ia4dd34ab152d1cc1d1658ebe957da7c3b8d32c06
Reviewed-on: http://review.couchbase.org/63366
Well-Formed: buildbot <build@couchbase.com>
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Will Gardner <will.gardner@couchbase.com>
src/ep_engine.cc

index bfb2510..3e6c08f 100644 (file)
@@ -4223,7 +4223,8 @@ ENGINE_ERROR_CODE EventuallyPersistentEngine::doSeqnoStats(const void *cookie,
 
         uint64_t relHighSeqno = vb->getHighSeqno();
 
-        ReaderLockHolder rlh(vb->getStateLock());
+        // An atomic read of vbucket state without acquiring the
+        // reader lock for state should suffice here.
         if (vb->getState() != vbucket_state_active) {
             uint64_t snapshot_start, snapshot_end;
             vb->getCurrentSnapshot(snapshot_start, snapshot_end);
@@ -4249,7 +4250,9 @@ ENGINE_ERROR_CODE EventuallyPersistentEngine::doSeqnoStats(const void *cookie,
         RCPtr<VBucket> vb = getVBucket(*itr);
         if (vb) {
             uint64_t relHighSeqno = vb->getHighSeqno();
-            ReaderLockHolder rlh(vb->getStateLock());
+
+            // An atomic read of vbucket state without acquiring the
+            // reader lock for state should suffice here.
             if (vb->getState() != vbucket_state_active) {
                 uint64_t snapshot_start, snapshot_end;
                 vb->getCurrentSnapshot(snapshot_start, snapshot_end);