Skip to content

Commit

Permalink
MB-43028: [2/2] Make overhead tracking safe at VBucket destruction
Browse files Browse the repository at this point in the history
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<void (long)>::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 <[email protected]>
Well-Formed: Build Bot <[email protected]>
Tested-by: Build Bot <[email protected]>
  • Loading branch information
jameseh96 authored and daverigby committed Dec 4, 2020
1 parent 6badce2 commit d169730
Showing 1 changed file with 4 additions and 4 deletions.
8 changes: 4 additions & 4 deletions engines/ep/src/ephemeral_bucket.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down

0 comments on commit d169730

Please sign in to comment.