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
main
Yi Wu 7 years ago committed by Facebook Github Bot
parent f47b4eeb1e
commit 6b3c71f6ed
  1. 5
      db/db_impl_compaction_flush.cc

@ -650,6 +650,8 @@ void DBImpl::NotifyOnCompactionCompleted(
if (shutting_down_.load(std::memory_order_acquire)) { if (shutting_down_.load(std::memory_order_acquire)) {
return; return;
} }
Version* current = cfd->current();
current->Ref();
// release lock while notifying events // release lock while notifying events
mutex_.Unlock(); mutex_.Unlock();
TEST_SYNC_POINT("DBImpl::NotifyOnCompactionCompleted::UnlockMutex"); TEST_SYNC_POINT("DBImpl::NotifyOnCompactionCompleted::UnlockMutex");
@ -672,7 +674,7 @@ void DBImpl::NotifyOnCompactionCompleted(
info.input_files.push_back(fn); info.input_files.push_back(fn);
if (info.table_properties.count(fn) == 0) { if (info.table_properties.count(fn) == 0) {
std::shared_ptr<const TableProperties> tp; std::shared_ptr<const TableProperties> tp;
auto s = cfd->current()->GetTableProperties(&tp, fmd, &fn); auto s = current->GetTableProperties(&tp, fmd, &fn);
if (s.ok()) { if (s.ok()) {
info.table_properties[fn] = tp; info.table_properties[fn] = tp;
} }
@ -689,6 +691,7 @@ void DBImpl::NotifyOnCompactionCompleted(
} }
} }
mutex_.Lock(); mutex_.Lock();
current->Unref();
// no need to signal bg_cv_ as it will be signaled at the end of the // no need to signal bg_cv_ as it will be signaled at the end of the
// flush process. // flush process.
#endif // ROCKSDB_LITE #endif // ROCKSDB_LITE

Loading…
Cancel
Save