From 6b3c71f6ed40236b33d1fc3cf3db7b99a29f87eb Mon Sep 17 00:00:00 2001 From: Yi Wu Date: Fri, 15 Sep 2017 11:45:33 -0700 Subject: [PATCH] Fix DBImpl::NotifyOnCompactionCompleted data race Summary: Access of `cfd->current()` needs to hold db mutex. The data race is caught by TSAN but hard to reproduce: https://gist.github.com/yiwu-arbug/0fc6dc0de915297a1740aa9610be9373 Closes https://github.com/facebook/rocksdb/pull/2894 Differential Revision: D5843884 Pulled By: yiwu-arbug fbshipit-source-id: 0a30a421bc96f51840821538ad6453dc0815a942 --- db/db_impl_compaction_flush.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/db/db_impl_compaction_flush.cc b/db/db_impl_compaction_flush.cc index 277aee6ed..8106b3e2d 100644 --- a/db/db_impl_compaction_flush.cc +++ b/db/db_impl_compaction_flush.cc @@ -650,6 +650,8 @@ void DBImpl::NotifyOnCompactionCompleted( if (shutting_down_.load(std::memory_order_acquire)) { return; } + Version* current = cfd->current(); + current->Ref(); // release lock while notifying events mutex_.Unlock(); TEST_SYNC_POINT("DBImpl::NotifyOnCompactionCompleted::UnlockMutex"); @@ -672,7 +674,7 @@ void DBImpl::NotifyOnCompactionCompleted( info.input_files.push_back(fn); if (info.table_properties.count(fn) == 0) { std::shared_ptr tp; - auto s = cfd->current()->GetTableProperties(&tp, fmd, &fn); + auto s = current->GetTableProperties(&tp, fmd, &fn); if (s.ok()) { info.table_properties[fn] = tp; } @@ -689,6 +691,7 @@ void DBImpl::NotifyOnCompactionCompleted( } } mutex_.Lock(); + current->Unref(); // no need to signal bg_cv_ as it will be signaled at the end of the // flush process. #endif // ROCKSDB_LITE