Skip to content

Commit 6e38fe8

Browse files
abhinavdangetichiyoung
abhinavdangeti
authored and
chiyoung
committed
MB-19113: Address false positive lock inversion seen with test_mb16357
The inversion being pointed out by this test case is between the snapshot lock and the hash table lock and this scenario would never happen during regular operation, this is because the first thread points out that the vbucket is active while the second indicates that the vbucket is replica. These 2 operations can never occur simulataneously. ThreadSanitizer assumes that this is a lock inversion because the 2 operations are done by different threads (main_thread and dcp_thread). The fix here suppresses this thread sanitizer warning by getting rid of the dcp_thread, and instead have the main_thread perform the activity that the dcp thread is responsible for. WARNING: ThreadSanitizer: lock-order-inversion (potential deadlock) (pid=5899) Cycle in lock order graph: M21372 (0x7d780000f510) => M21408 (0x7d640000f920) => M21372 Mutex M21408 acquired here while holding mutex M21372 in main thread: #0 pthread_mutex_lock <null> (engine_testapp+0x00000047e970) #1 cb_mutex_enter <null> (libplatform.so.0.1.0+0x000000003870) #2 Mutex::acquire() /home/couchbase/couchbase/ep-engine/src/mutex.cc:31 (ep.so+0x0000001e287e) #3 LockHolder::lock() /home/couchbase/couchbase/ep-engine/src/locks.h:71 (ep.so+0x000000082543) #4 LockHolder::LockHolder(Mutex&, bool) /home/couchbase/couchbase/ep-engine/src/locks.h:48 (ep.so+0x0000000821b2) #5 VBucket::getSnapshotLock() /home/couchbase/couchbase/ep-engine/src/vbucket.h:212 (ep.so+0x000000104c72) #6 EventuallyPersistentStore::queueDirty(RCPtr<VBucket>&, StoredValue*, LockHolder*, bool, bool, bool) /home/couchbase/couchbase/ep-engine/src/ep.cc:2863 (ep.so+0x0000000d7123) #7 EventuallyPersistentStore::set(Item const&, void const*, bool, unsigned char) /home/couchbase/couchbase/ep-engine/src/ep.cc:683 (ep.so+0x0000000d9dfa) #8 EventuallyPersistentEngine::store(void const*, void*, unsigned long*, ENGINE_STORE_OPERATION, unsigned short) /home/couchbase/couchbase/ep-engine/src/ep_engine.cc:2128 (ep.so+0x00000013d538) #9 EvpStore(engine_interface*, void const*, void*, unsigned long*, ENGINE_STORE_OPERATION, unsigned short) /home/couchbase/couchbase/ep-engine/src/ep_engine.cc:229 (ep.so+0x00000013712d) #10 mock_store /home/couchbase/couchbase/memcached/programs/engine_testapp/engine_testapp.c (engine_testapp+0x0000004c7304) #11 storeCasVb11(engine_interface*, engine_interface_v1*, void const*, ENGINE_STORE_OPERATION, char const*, char const*, unsigned long, unsigned int, void**, unsigned long, unsigned short, unsigned int, unsigned char) /home/couchbase/couchbase/ep-engine/tests/ep_test_apis.cc:659 (ep_testsuite.so+0x0000000e8d17) #12 store(engine_interface*, engine_interface_v1*, void const*, ENGINE_STORE_OPERATION, char const*, char const*, void**, unsigned long, unsigned short, unsigned int, unsigned char) /home/couchbase/couchbase/ep-engine/tests/ep_test_apis.cc:631 (ep_testsuite.so+0x0000000e654a) #13 test_mb16357(engine_interface*, engine_interface_v1*) /home/couchbase/couchbase/ep-engine/tests/ep_testsuite.cc:11713 (ep_testsuite.so+0x0000000afc36) #14 execute_test /home/couchbase/couchbase/memcached/programs/engine_testapp/engine_testapp.c (engine_testapp+0x0000004c4e2f) #15 main crtstuff.c (engine_testapp+0x0000004c2d91) Mutex M21372 acquired here while holding mutex M21408 in thread T10: #0 pthread_mutex_lock <null> (engine_testapp+0x00000047e970) #1 cb_mutex_enter <null> (libplatform.so.0.1.0+0x000000003870) #2 Mutex::acquire() /home/couchbase/couchbase/ep-engine/src/mutex.cc:31 (ep.so+0x0000001e287e) #3 LockHolder::lock() /home/couchbase/couchbase/ep-engine/src/locks.h:71 (ep.so+0x000000082543) #4 LockHolder::LockHolder(Mutex&, bool) /home/couchbase/couchbase/ep-engine/src/locks.h:48 (ep.so+0x0000000821b2) #5 HashTable::getLockedBucket(int, int*) /home/couchbase/couchbase/ep-engine/src/stored-value.h:1266 (ep.so+0x00000008418a) #6 HashTable::getLockedBucket(std::string const&, int*) /home/couchbase/couchbase/ep-engine/src/stored-value.h:1295 (ep.so+0x00000007df9b) #7 EventuallyPersistentStore::setWithMeta(Item const&, unsigned long, void const*, bool, bool, unsigned char, bool, bool) /home/couchbase/couchbase/ep-engine/src/ep.cc:1827 (ep.so+0x0000000e6b4f) #8 PassiveStream::commitMutation(MutationResponse*, bool) /home/couchbase/couchbase/ep-engine/src/dcp-stream.cc:1369 (ep.so+0x00000029ba21) #9 PassiveStream::processMutation(MutationResponse*) /home/couchbase/couchbase/ep-engine/src/dcp-stream.cc:1341 (ep.so+0x00000029a7a0) #10 PassiveStream::processBufferedMessages(unsigned int&) /home/couchbase/couchbase/ep-engine/src/dcp-stream.cc:1281 (ep.so+0x00000029a0f2) #11 DcpConsumer::processBufferedItems() /home/couchbase/couchbase/ep-engine/src/dcp-consumer.cc:599 (ep.so+0x000000262a23) #12 Processer::run() /home/couchbase/couchbase/ep-engine/src/dcp-consumer.cc:48 (ep.so+0x0000002625ff) #13 ExecutorThread::run() /home/couchbase/couchbase/ep-engine/src/executorthread.cc:110 (ep.so+0x0000001e3dd9) #14 launch_executor_thread(void*) /home/couchbase/couchbase/ep-engine/src/executorthread.cc:34 (ep.so+0x0000001e32ea) #15 platform_thread_wrap /home/couchbase/couchbase/platform/src/cb_pthreads.c (libplatform.so.0.1.0+0x00000000362c) Change-Id: I6c7b1fadf76529a044341a4a9b6ed0ea829c4999 Reviewed-on: http://review.couchbase.org/62567 Well-Formed: buildbot <build@couchbase.com> Tested-by: buildbot <build@couchbase.com> Reviewed-by: Dave Rigby <daver@couchbase.com> Reviewed-by: Chiyoung Seo <chiyoung@couchbase.com>
1 parent 771859f commit 6e38fe8

1 file changed

Lines changed: 48 additions & 57 deletions

File tree

‎tests/ep_testsuite.cc‎

Lines changed: 48 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -7543,7 +7543,7 @@ static enum test_result test_all_keys_api(ENGINE_HANDLE *h, ENGINE_HANDLE_V1 *h1
75437543
protocol_binary_request_header *pkt1 =
75447544
createPacket(CMD_GET_KEYS, 0, 0, ext, extlen,
75457545
"key_10", keylen, NULL, 0, 0x00);
7546-
delete ext;
7546+
delete[] ext;
75477547

75487548
check(h1->unknown_command(h, NULL, pkt1, add_response) == ENGINE_SUCCESS,
75497549
"Failed to get all_keys, sort: ascending");
@@ -11643,57 +11643,6 @@ struct mb16357_ctx {
1164311643
};
1164411644

1164511645
extern "C" {
11646-
static void dcp_thread_func(void *args) {
11647-
struct mb16357_ctx *ctx = static_cast<mb16357_ctx *>(args);
11648-
11649-
const void *cookie = testHarness.create_cookie();
11650-
uint32_t opaque = 0xFFFF0000;
11651-
uint32_t flags = 0;
11652-
std::string name = "unittest";
11653-
11654-
while (get_int_stat(ctx->h, ctx->h1, "ep_pending_compactions") == 0);
11655-
11656-
// Switch to replica
11657-
check(set_vbucket_state(ctx->h, ctx->h1, 0, vbucket_state_replica),
11658-
"Failed to set vbucket state.");
11659-
11660-
// Open consumer connection
11661-
checkeq(ctx->h1->dcp.open(ctx->h, cookie, opaque, 0, flags,
11662-
(void*)name.c_str(), name.length()),
11663-
ENGINE_SUCCESS,
11664-
"Failed dcp Consumer open connection.");
11665-
11666-
add_stream_for_consumer(ctx->h, ctx->h1, cookie, opaque++, 0, 0,
11667-
PROTOCOL_BINARY_RESPONSE_SUCCESS);
11668-
11669-
11670-
uint32_t stream_opaque = get_int_stat(ctx->h, ctx->h1,
11671-
"eq_dcpq:unittest:stream_0_opaque",
11672-
"dcp");
11673-
11674-
11675-
for (int i = 1; i <= ctx->items; i++) {
11676-
std::stringstream ss;
11677-
ss << "kamakeey-" << i;
11678-
11679-
// send mutations in single mutation snapshots to race more with compaction
11680-
checkeq(ctx->h1->dcp.snapshot_marker(ctx->h, cookie,
11681-
stream_opaque, 0/*vbid*/,
11682-
ctx->items, ctx->items + i, 2),
11683-
ENGINE_SUCCESS,
11684-
"Failed to send snapshot marker");
11685-
checkeq(ctx->h1->dcp.mutation(ctx->h, cookie, stream_opaque,
11686-
ss.str().c_str(), ss.str().length(),
11687-
"value", 5, i * 3, 0, 0, 0,
11688-
i + ctx->items, i + ctx->items,
11689-
0, 0, "", 0, INITIAL_NRU_VALUE),
11690-
ENGINE_SUCCESS,
11691-
"Failed to send dcp mutation");
11692-
}
11693-
11694-
testHarness.destroy_cookie(cookie);
11695-
}
11696-
1169711646
static void compact_thread_func(void *args) {
1169811647
struct mb16357_ctx *ctx = static_cast<mb16357_ctx *>(args);
1169911648
compact_db(ctx->h, ctx->h1, 0, 99, ctx->items, 1);
@@ -11721,17 +11670,59 @@ static enum test_result test_mb16357(ENGINE_HANDLE *h,
1172111670
testHarness.time_travel(3617); // force expiry pushing time forward.
1172211671

1172311672
struct mb16357_ctx ctx(h, h1, num_items);
11724-
cb_thread_t cp_thread, dcp_thread;
11673+
cb_thread_t cp_thread;
1172511674

1172611675
cb_assert(cb_create_thread(&cp_thread,
1172711676
compact_thread_func,
1172811677
&ctx, 0) == 0);
11729-
cb_assert(cb_create_thread(&dcp_thread,
11730-
dcp_thread_func,
11731-
&ctx, 0) == 0);
11678+
11679+
const void *cookie = testHarness.create_cookie();
11680+
uint32_t opaque = 0xFFFF0000;
11681+
uint32_t flags = 0;
11682+
std::string name = "unittest";
11683+
11684+
while (get_int_stat(h, h1, "ep_pending_compactions") == 0);
11685+
11686+
// Switch to replica
11687+
check(set_vbucket_state(h, h1, 0, vbucket_state_replica),
11688+
"Failed to set vbucket state.");
11689+
11690+
// Open consumer connection
11691+
checkeq(h1->dcp.open(h, cookie, opaque, 0, flags,
11692+
(void*)name.c_str(), name.length()),
11693+
ENGINE_SUCCESS,
11694+
"Failed dcp Consumer open connection.");
11695+
11696+
add_stream_for_consumer(h, h1, cookie, opaque++, 0, 0,
11697+
PROTOCOL_BINARY_RESPONSE_SUCCESS);
11698+
11699+
uint32_t stream_opaque = get_int_stat(h, h1,
11700+
"eq_dcpq:unittest:stream_0_opaque",
11701+
"dcp");
11702+
11703+
11704+
for (int i = 1; i <= num_items; i++) {
11705+
std::stringstream ss;
11706+
ss << "kamakeey-" << i;
11707+
11708+
// send mutations in single mutation snapshots to race more with compaction
11709+
checkeq(h1->dcp.snapshot_marker(h, cookie,
11710+
stream_opaque, 0/*vbid*/,
11711+
num_items, num_items + i, 2),
11712+
ENGINE_SUCCESS,
11713+
"Failed to send snapshot marker");
11714+
checkeq(h1->dcp.mutation(h, cookie, stream_opaque,
11715+
ss.str().c_str(), ss.str().length(),
11716+
"value", 5, i * 3, 0, 0, 0,
11717+
i + num_items, i + num_items,
11718+
0, 0, "", 0, INITIAL_NRU_VALUE),
11719+
ENGINE_SUCCESS,
11720+
"Failed to send dcp mutation");
11721+
}
11722+
11723+
testHarness.destroy_cookie(cookie);
1173211724

1173311725
cb_assert(cb_join_thread(cp_thread) == 0);
11734-
cb_assert(cb_join_thread(dcp_thread) == 0);
1173511726

1173611727
return SUCCESS;
1173711728
}

0 commit comments

Comments
 (0)