From 349542332a8fbfef3bcc9f6ace013919f2becd98 Mon Sep 17 00:00:00 2001 From: Maysam Yabandeh Date: Thu, 13 Dec 2018 19:22:07 -0800 Subject: [PATCH] Fix race condition on options_file_number_ (#4780) Summary: options_file_number_ must be written under db::mutex_ sine its read is protected by mutex_ in ::GetLiveFiles(). However currently it is written in ::RenameTempFileToOptionsFile() which according to its contract must be called without holding db::mutex_. The patch fixes the race condition by also acquitting the mutex_ before writing options_file_number_. Also it does that only if the rename of option file is successful. Pull Request resolved: https://github.com/facebook/rocksdb/pull/4780 Differential Revision: D13461411 Pulled By: maysamyabandeh fbshipit-source-id: 2d5bae96a1f3e969ef2505b737cf2d7ae749787b --- db/db_impl.cc | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/db/db_impl.cc b/db/db_impl.cc index e22ce20a43..78ade2c8be 100644 --- a/db/db_impl.cc +++ b/db/db_impl.cc @@ -2863,11 +2863,15 @@ Status DBImpl::RenameTempFileToOptionsFile(const std::string& file_name) { #ifndef ROCKSDB_LITE Status s; - versions_->options_file_number_ = versions_->NewFileNumber(); + uint64_t options_file_number = versions_->NewFileNumber(); std::string options_file_name = - OptionsFileName(GetName(), versions_->options_file_number_); + OptionsFileName(GetName(), options_file_number); // Retry if the file name happen to conflict with an existing one. s = GetEnv()->RenameFile(file_name, options_file_name); + if (s.ok()) { + InstrumentedMutexLock l(&mutex_); + versions_->options_file_number_ = options_file_number; + } if (0 == disable_delete_obsolete_files_) { DeleteObsoleteOptionsFiles();