Fix data race #1

Summary:
This is first in a series of diffs that fixes data races detected by thread sanitizer.

Here the problem is that we call Ref() on a column family during a single-threaded write, without holding a mutex.

Test Plan: TSAN is no longer complaining about LevelLimitReopen.

Reviewers: yhchiang, rven, sdong

Reviewed By: sdong

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D32121

db/column_family.cc

@@ -299,7 +299,7 @@ ColumnFamilyData::ColumnFamilyData(
 
 // DB mutex held
 ColumnFamilyData::~ColumnFamilyData() {
-  assert(refs_ == 0);
+  assert(refs_.load(std::memory_order_relaxed) == 0);
   // remove from linked list
   auto prev = prev_;
   auto next = next_;
@@ -731,7 +731,7 @@ ColumnFamilyData* ColumnFamilySet::CreateColumnFamily(
 void ColumnFamilySet::FreeDeadColumnFamilies() {
   autovector<ColumnFamilyData*> to_delete;
   for (auto cfd = dummy_cfd_->next_; cfd != dummy_cfd_; cfd = cfd->next_) {
-    if (cfd->refs_ == 0) {
+    if (cfd->refs_.load(std::memory_order_relaxed) == 0) {
       to_delete.push_back(cfd);
     }
   }

db/column_family.h

@@ -134,14 +134,18 @@ class ColumnFamilyData {
   // thread-safe
   const std::string& GetName() const { return name_; }
 
-  void Ref() { ++refs_; }
+  // Ref() can only be called whily holding a DB mutex or during a
+  // single-threaded write.
+  void Ref() { refs_.fetch_add(1, std::memory_order_relaxed); }
   // will just decrease reference count to 0, but will not delete it. returns
   // true if the ref count was decreased to zero. in that case, it can be
   // deleted by the caller immediately, or later, by calling
   // FreeDeadColumnFamilies()
+  // Unref() can only be called while holding a DB mutex
   bool Unref() {
-    assert(refs_ > 0);
+    int old_refs = refs_.fetch_sub(1, std::memory_order_relaxed);
-    return --refs_ == 0;
+    assert(old_refs > 0);
+    return old_refs == 1;
   }
 
   // SetDropped() can only be called under following conditions:
@@ -290,7 +294,7 @@ class ColumnFamilyData {
   Version* dummy_versions_;  // Head of circular doubly-linked list of versions.
   Version* current_;         // == dummy_versions->prev_
 
-  int refs_;                   // outstanding references to ColumnFamilyData
+  std::atomic<int> refs_;      // outstanding references to ColumnFamilyData
   bool dropped_;               // true if client dropped it
 
   const InternalKeyComparator internal_comparator_;
@@ -373,7 +377,8 @@ class ColumnFamilySet {
       // dummy is never dead or dropped, so this will never be infinite
       do {
         current_ = current_->next_;
-      } while (current_->refs_ == 0 || current_->IsDropped());
+      } while (current_->refs_.load(std::memory_order_relaxed) == 0 ||
+               current_->IsDropped());
       return *this;
     }
     bool operator!=(const iterator& other) {

db/flush_scheduler.h

@@ -23,6 +23,7 @@ class FlushScheduler {
 
   void ScheduleFlush(ColumnFamilyData* cfd);
   // Returns Ref()-ed column family. Client needs to Unref()
+  // REQUIRES: db mutex is held (exception is single-threaded recovery)
   ColumnFamilyData* GetNextColumnFamily();
 
   bool Empty();