From d16973029d7e009cb9ca5f4df46bac8f31cb8ab1 Mon Sep 17 00:00:00 2001 From: James Harrison <00jamesh@gmail.com> Date: Fri, 4 Dec 2020 12:53:06 +0000 Subject: [PATCH] MB-43028: [2/2] Make overhead tracking safe at VBucket destruction Merging http://review.couchbase.org/c/kv_engine/+/136495 into master uncovered santizer issues (mad-hatter CV runs an older Clang and did not identify these issues). This patch resolves one of these issues, _before_ the above patch is merged to master. UndefinedBehaviorSanitizer: undefined-behavior ../kv_engine/engines/ep/src/ephemeral_bucket.cc:303:27 runtime error: member access within address 0x6160007fd780 which does not point to an object of type 'KVBucket' #0 0x7fa90ca0c9bd in EphemeralBucket::makeVBucket(...)::$_3::operator()(long) const /home/couchbase/jenkins/workspace/kv_engine.ASan-UBSan_master/build/../kv_engine/engines/ep/src/ephemeral_bucket.cc:303:27 #1 0x7fa90c620aac in std::function::operator()(long) const /usr/bin/../lib/gcc/x86_64-linux-gnu/7.5.0/../../../../include/c++/7.5.0/bits/std_function.h:706:14 #2 0x7fa90c61a036 in Checkpoint::~Checkpoint() /home/couchbase/jenkins/workspace/kv_engine.ASan-UBSan_master/build/../kv_engine/engines/ep/src/checkpoint.cc:224:5 The callback captures a pointer to the EphemeralBucket which created the VBucket, in order to use the EPStats instance. However, the EphemeralBucket may be destroyed before the VBucket, making this unsafe. Capture stats by reference directly to avoid this. Change-Id: Ide06432d4229a13bc79e21ab6484eca036ea3f75 Reviewed-on: http://review.couchbase.org/c/kv_engine/+/141493 Reviewed-by: Dave Rigby Well-Formed: Build Bot Tested-by: Build Bot --- engines/ep/src/ephemeral_bucket.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/engines/ep/src/ephemeral_bucket.cc b/engines/ep/src/ephemeral_bucket.cc index ab3d265b71..e82982375c 100644 --- a/engines/ep/src/ephemeral_bucket.cc +++ b/engines/ep/src/ephemeral_bucket.cc @@ -289,15 +289,15 @@ VBucketPtr EphemeralBucket::makeVBucket( mightContainXattrs, replicationTopology); - vb->ht.setMemChangedCallback([this, vb](int64_t delta) { + vb->ht.setMemChangedCallback([vb, &stats = stats](int64_t delta) { if (vb->getState() == vbucket_state_replica) { - this->stats.replicaHTMemory += delta; + stats.replicaHTMemory += delta; } }); vb->checkpointManager->setOverheadChangedCallback( - [this, vb](int64_t delta) { + [vb, &stats = stats](int64_t delta) { if (vb->getState() == vbucket_state_replica) { - this->stats.replicaCheckpointOverhead += delta; + stats.replicaCheckpointOverhead += delta; } }); return VBucketPtr(vb, VBucket::DeferredDeleter(engine));