hazptr: Fix hazptr_priv dtor not to accept new retired objects

Summary: When hazptr_priv dtor pushes objects to the domain and that happens to trigger (either due to timed cleanup or reaching the bulk reclaim threshold) bulk reclamation that retires objects. These newly retired objects should be pushed to the domain and not into the hazptr_priv.

Reviewed By: djwatson

Differential Revision: D8410795

fbshipit-source-id: c7af01cc41a1d82896bd3700fa072f698c0aa77b

folly/synchronization/HazptrThrLocal.h

@@ -166,11 +166,13 @@ class hazptr_priv {
   Atom<hazptr_obj<Atom>*> head_;
   Atom<hazptr_obj<Atom>*> tail_;
   int rcount_;
+  bool in_dtor_;
 
  public:
-  hazptr_priv() : head_(nullptr), tail_(nullptr), rcount_(0) {}
+  hazptr_priv() : head_(nullptr), tail_(nullptr), rcount_(0), in_dtor_(false) {}
 
   ~hazptr_priv() {
+    in_dtor_ = true;
     if (!empty()) {
       push_all_to_domain();
     }
@@ -185,6 +187,15 @@ class hazptr_priv {
   }
 
   void push(hazptr_obj<Atom>* obj) {
+    if (!in_dtor_) {
+      push_in_priv_list(obj);
+    } else {
+      hazptr_obj_list<Atom> l(obj);
+      hazptr_domain_push_retired<Atom>(l);
+    }
+  }
+
+  void push_in_priv_list(hazptr_obj<Atom>* obj) {
     while (true) {
       if (tail()) {
         if (push_in_non_empty_list(obj)) {

folly/synchronization/test/HazptrTest.cpp

@@ -780,6 +780,29 @@ void cleanup_test() {
   }
 }
 
+template <template <typename> class Atom = std::atomic>
+void priv_dtor_test() {
+  c_.clear();
+  using NodeT = NodeRC<true, Atom>;
+  auto y = new NodeT;
+  y->acquire_link_safe();
+  struct Foo : hazptr_obj_base<Foo, Atom> {
+    hazptr_root<NodeT, Atom> r_;
+  };
+  auto x = new Foo;
+  x->r_().store(y);
+  /* Thread retires x. Dtor of TLS priv list pushes x to domain, which
+     triggers bulk reclaim due to timed cleanup (when the test is run
+     by itself). Reclamation of x unlinks and retires y. y should
+     not be pushed into the thread's priv list. It should be pushed to
+     domain instead. */
+  auto thr = DSched::thread([&]() { x->retire(); });
+  DSched::join(thr);
+  ASSERT_EQ(c_.ctors(), 1);
+  hazptr_cleanup<Atom>();
+  ASSERT_EQ(c_.dtors(), 1);
+}
+
 template <template <typename> class Atom = std::atomic>
 void lifo_test() {
   for (int i = 0; i < FLAGS_num_reps; ++i) {
@@ -1019,6 +1042,15 @@ TEST(HazptrTest, dsched_cleanup) {
   cleanup_test<DeterministicAtomic>();
 }
 
+TEST(HazptrTest, priv_dtor) {
+  priv_dtor_test();
+}
+
+TEST(HazptrTest, dsched_priv_dtor) {
+  DSched sched(DSched::uniform(0));
+  priv_dtor_test<DeterministicAtomic>();
+}
+
 TEST(HazptrTest, lifo) {
   lifo_test();
 }