hazptr_obj_cohort: Fix reclamation of safe list

Summary:
Fix a bug in hazptr_obj_cohort handling of children of reclaimed nodes during reclamation of the safe list. When the cohort is active, the children nodes should be pushed to the domain (to be checked against hazard pointers) and not reclaimed immediately because it is possible that a child node is protected by hazard pointers.

Added test that catches the bug.

Reviewed By: yfeldblum

Differential Revision: D30170939

fbshipit-source-id: e5bfe46b6fa5d4e5ce5cef02a3934ced424138f0

folly/synchronization/HazptrObj.h

@@ -374,7 +374,13 @@ class hazptr_obj_cohort {
         (*(obj->reclaim()))(obj, children);
         obj = next;
       }
-      obj = children.head();
+      if (!children.empty()) {
+        if (active()) {
+          hazptr_domain_push_list<Atom>(children);
+        } else {
+          obj = children.head();
+        }
+      }
     }
   }
 

folly/synchronization/test/HazptrTest.cpp

@@ -875,6 +875,53 @@ void recursive_destruction_test() {
   ASSERT_EQ(c_.dtors(), total);
 }
 
+// Used in cohort_safe_list_children_test
+struct NodeA : hazptr_obj_base_linked<NodeA> {
+  std::atomic<NodeA*> next_{nullptr};
+  int& sum_;
+  int val_;
+
+  NodeA(hazptr_obj_cohort<>& coh, int& sum, int v = 0) : sum_(sum), val_(v) {
+    this->set_cohort_tag(&coh);
+  }
+  ~NodeA() { sum_ += val_; }
+  void set_next(NodeA* ptr) { next_.store(ptr); }
+  template <typename S>
+  void push_links(bool m, S& s) {
+    if (m) {
+      auto p = next_.load();
+      if (p) {
+        s.push(p);
+      }
+    }
+  }
+};
+
+void cohort_safe_list_children_test() {
+  int sum = 0;
+  hazptr_obj_cohort<> cohort;
+  NodeA* p1 = new NodeA(cohort, sum, 1000);
+  NodeA* p2 = new NodeA(cohort, sum, 2000);
+  p2->acquire_link_safe();
+  p1->set_next(p2); // p2 is p1's child
+  hazard_pointer<> h = make_hazard_pointer<>();
+  h.reset_protection(p2);
+  p1->retire();
+  /* When p1 is retired, it is inserted into cohort, then pushed into
+     the domain's tagged list, then when p1 is found unprotected by
+     hazard pointers it will be pushed into cohort's safe list. When
+     p1 is reclaimed, p2 (p1's child) will be automatically retired to
+     the domain's tagged list. */
+  /* Retire enough nodes to trigger asynchronous reclamation to
+     reclaim p1. */
+  for (int i = 0; i < 1100; ++i) {
+    NodeA* p = new NodeA(cohort, sum);
+    p->retire();
+  }
+  /* At this point p1 should be reclaimed but not p2 */
+  DCHECK_EQ(sum, 1000);
+}
+
 void fork_test() {
   folly::enable_hazptr_thread_pool_executor();
   auto trigger_reclamation = [] {
@@ -1180,6 +1227,10 @@ TEST(HazptrTest, dsched_recursive_destruction) {
   recursive_destruction_test<DeterministicAtomic>();
 }
 
+TEST(HazptrTest, cohort_safe_list_children) {
+  cohort_safe_list_children_test();
+}
+
 TEST(HazptrTest, fork) {
   fork_test();
 }