From 46fde6b65336c5e176d347b80422fd5f6a7ae7c2 Mon Sep 17 00:00:00 2001 From: Maysam Yabandeh Date: Wed, 16 May 2018 12:51:31 -0700 Subject: [PATCH] Fix race condition between log_.erase and log_.back Summary: log_ contract specifies that it should not be modified unless both mutex_ and log_write_mutex_ are held. log_.erase however does that with only holding mutex_. This causes a race condition with two_write_queues since logs_.back is read with holding only log_write_mutex_ (which is correct according to logs_ contract) but logs_.erase is called concurrently. This is probably the cause of logs_.back returning nullptr in https://github.com/facebook/rocksdb/issues/3852 although I could not reproduce it. Fixes https://github.com/facebook/rocksdb/issues/3852 Closes https://github.com/facebook/rocksdb/pull/3859 Differential Revision: D8026103 Pulled By: maysamyabandeh fbshipit-source-id: ee394e00fe4aa520d884c5ef87981e9d6b5ccb28 --- db/db_impl.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/db/db_impl.cc b/db/db_impl.cc index c833ecacd..cc1140160 100644 --- a/db/db_impl.cc +++ b/db/db_impl.cc @@ -810,6 +810,8 @@ void DBImpl::MarkLogsSynced(uint64_t up_to, bool synced_dir, assert(log.getting_synced); if (status.ok() && logs_.size() > 1) { logs_to_free_.push_back(log.ReleaseWriter()); + // To modify logs_ both mutex_ and log_write_mutex_ must be held + InstrumentedMutexLock l(&log_write_mutex_); it = logs_.erase(it); } else { log.getting_synced = false;