Domain destruction fixes

Summary:
If a retired object's destructor retire()s other hazard pointers, currently
these are not cleaned up correctly when the domain destructs.

Retired pointers must be cleaned up before destroying hazptr_recs, and must be done
iteratively until no more garbage is generated.

Reviewed By: magedm

Differential Revision: D4987333

fbshipit-source-id: bcdd61abb47caca0892a8c4dbb864d17d4f2fa30

diff --git a/folly/experimental/hazptr/hazptr-impl.h b/folly/experimental/hazptr/hazptr-impl.h
index ecf6865d972cbaa607cb0a2fa209c9148ca1b581..50874df775b4b287891b2dbc57c6ea5c7bfc1bb2 100644 (file)
--- a/folly/experimental/hazptr/hazptr-impl.h
+++ b/folly/experimental/hazptr/hazptr-impl.h
@@ -183,6 +183,17 @@ inline const void* hazptr_obj::getObjPtr() const {
 
 inline hazptr_domain::~hazptr_domain() {
   DEBUG_PRINT(this);
 
 inline hazptr_domain::~hazptr_domain() {
   DEBUG_PRINT(this);

+  { /* reclaim all remaining retired objects */
+    hazptr_obj* next;
+    auto retired = retired_.exchange(nullptr);
+    while (retired) {
+      for (auto p = retired; p; p = next) {
+        next = p->next_;
+        (*(p->reclaim_))(p);
+      }
+      retired = retired_.exchange(nullptr);
+    }
+  }

   { /* free all hazptr_rec-s */
     hazptr_rec* next;
     for (auto p = hazptrs_.load(); p; p = next) {
   { /* free all hazptr_rec-s */
     hazptr_rec* next;
     for (auto p = hazptrs_.load(); p; p = next) {


@@ -190,13 +201,6 @@ inline hazptr_domain::~hazptr_domain() {
       mr_->deallocate(static_cast<void*>(p), sizeof(hazptr_rec));
     }
   }
       mr_->deallocate(static_cast<void*>(p), sizeof(hazptr_rec));
     }
   }

-  { /* reclaim all remaining retired objects */
-    hazptr_obj* next;
-    for (auto p = retired_.load(); p; p = next) {
-      next = p->next_;
-      (*(p->reclaim_))(p);
-    }
-  }

 }
 
 inline void hazptr_domain::try_reclaim() {
 }
 
 inline void hazptr_domain::try_reclaim() {

diff --git a/folly/experimental/hazptr/test/HazptrTest.cpp b/folly/experimental/hazptr/test/HazptrTest.cpp
index fa4ac1ec2f196f0508efa1babd9c0f6973e7c048..205052620cd312665dacc70e6ea774d9253f09f3 100644 (file)
--- a/folly/experimental/hazptr/test/HazptrTest.cpp
+++ b/folly/experimental/hazptr/test/HazptrTest.cpp
@@ -256,3 +256,22 @@ TEST_F(HazptrTest, VirtualTest) {
     EXPECT_EQ(bar->a, i);
   }
 }
     EXPECT_EQ(bar->a, i);
   }
 }

+
+TEST_F(HazptrTest, DestructionTest) {
+  hazptr_domain myDomain0;
+  struct Thing : public hazptr_obj_base<Thing> {
+    Thing* next;
+    Thing(Thing* n) : next(n) {}
+    ~Thing() {
+      DEBUG_PRINT("this: " << this << " next: " << next);
+      if (next) {
+        next->retire();
+      }
+    }
+  };
+  Thing* last{nullptr};
+  for (int i = 0; i < 2000; i++) {
+    last = new Thing(last);
+  }
+  last->retire();
+}