hazptr: Fix hazptr_cleanup

Summary:
Added missing steps for registering and deregistering per-thread hazptr_priv structures with the default domain.
Updated tests to detect the error

Reviewed By: djwatson, davidtgoldblatt

Differential Revision: D6965459

fbshipit-source-id: 6eb6a5981c1d22518f318ace7387a3941a013352

folly/experimental/hazptr/hazptr-impl.h

@@ -183,6 +183,7 @@ class hazptr_priv {
     tail_ = nullptr;
     rcount_ = 0;
     active_ = true;
+    default_hazptr_domain().priv_add(this);
   }
 
   bool active() {
@@ -932,6 +933,14 @@ inline void hazptr_domain::cleanup() {
   bulkReclaim();
 }
 
+inline void hazptr_domain::priv_add(hazptr_priv* rec) {
+  priv_.insert(rec);
+}
+
+inline void hazptr_domain::priv_remove(hazptr_priv* rec) {
+  priv_.remove(rec);
+}
+
 inline hazptr_rec* hazptr_domain::hazptrAcquire() {
   hazptr_rec* p;
   hazptr_rec* next;
@@ -1174,7 +1183,6 @@ FOLLY_ALWAYS_INLINE hazptr_tc* hazptr_tc_tls() {
 }
 
 inline void hazptr_tc_init() {
-  HAZPTR_DEBUG_PRINT("");
   auto& tc = tls_tc_data_;
   HAZPTR_DEBUG_PRINT(&tc);
   tc.count_ = 0;
@@ -1231,6 +1239,7 @@ inline void hazptr_priv_shutdown() {
   if (!priv.empty()) {
     priv.push_all_to_domain();
   }
+  default_hazptr_domain().priv_remove(&priv);
 }
 
 inline bool hazptr_priv_try_retire(hazptr_obj* obj) {

folly/experimental/hazptr/hazptr.h

@@ -90,6 +90,8 @@ class hazptr_domain {
   template <typename T, typename D = std::default_delete<T>>
   void retire(T* obj, D reclaim = {});
   void cleanup();
+  void priv_add(hazptr_priv* rec);
+  void priv_remove(hazptr_priv* rec);
 
  private:
   friend class hazptr_obj_batch;

folly/experimental/hazptr/test/HazptrTest.cpp

@@ -33,7 +33,7 @@
 
 DEFINE_int32(num_threads, 5, "Number of threads");
 DEFINE_int64(num_reps, 1, "Number of test reps");
-DEFINE_int64(num_ops, 10, "Number of ops or pairs of ops per rep");
+DEFINE_int64(num_ops, 1007, "Number of ops or pairs of ops per rep");
 
 using namespace folly::hazptr;
 
@@ -443,11 +443,11 @@ struct Foo : hazptr_obj_base_refcounted<Foo> {
   Foo* next_;
   Foo(int v, Foo* n) : val_(v), marked_(false), next_(n) {
     HAZPTR_DEBUG_PRINT("");
-    ++constructed;
+    constructed.fetch_add(1);
   }
   ~Foo() {
     HAZPTR_DEBUG_PRINT("");
-    ++destroyed;
+    destroyed.fetch_add(1);
     if (marked_) {
       return;
     }
@@ -604,21 +604,39 @@ TEST_F(HazptrTest, FreeFunctionRetire) {
 
 TEST_F(HazptrTest, FreeFunctionCleanup) {
   CHECK_GT(FLAGS_num_threads, 0);
+  int threadOps = 1007;
+  int mainOps = 19;
   constructed.store(0);
   destroyed.store(0);
+  std::atomic<int> threadsDone{0};
+  std::atomic<bool> mainDone{false};
   std::vector<std::thread> threads(FLAGS_num_threads);
   for (int tid = 0; tid < FLAGS_num_threads; ++tid) {
     threads[tid] = std::thread([&, tid]() {
-      for (int j = tid; j < FLAGS_num_ops; j += FLAGS_num_threads) {
+      for (int j = tid; j < threadOps; j += FLAGS_num_threads) {
         auto p = new Foo(j, nullptr);
         p->retire();
       }
+      threadsDone.fetch_add(1);
+      while (!mainDone.load()) {
+        /* spin */;
+      }
     });
   }
+  { // include the main thread in the test
+    for (int i = 0; i < mainOps; ++i) {
+      auto p = new Foo(0, nullptr);
+      p->retire();
+    }
+  }
+  while (threadsDone.load() < FLAGS_num_threads) {
+    /* spin */;
+  }
+  CHECK_EQ(constructed.load(), threadOps + mainOps);
+  hazptr_cleanup();
+  CHECK_EQ(destroyed.load(), threadOps + mainOps);
+  mainDone.store(true);
   for (auto& t : threads) {
     t.join();
   }
-  CHECK_EQ(constructed.load(), FLAGS_num_ops);
-  hazptr_cleanup();
-  CHECK_EQ(destroyed.load(), FLAGS_num_ops);
 }