Hazard pointers: Fix leak in hazptr_priv destruction
authorMaged Michael <magedmichael@fb.com>
Fri, 30 Jun 2017 04:21:32 +0000 (21:21 -0700)
committerFacebook Github Bot <facebook-github-bot@users.noreply.github.com>
Fri, 30 Jun 2017 04:22:02 +0000 (21:22 -0700)
Summary:
- Fixed leak in hazptr_priv destruction
- Updated tests to detect leak

Reviewed By: djwatson

Differential Revision: D5351280

fbshipit-source-id: 724810cbbe0565f0cbd41f9d2121abefd98487bd

folly/experimental/hazptr/hazptr-impl.h
folly/experimental/hazptr/test/HazptrTest.cpp

index 6b19bd8aec8d0a3adf9729bac33007e173c6bf83..70123659053dedcd5133cbfb6fa1b6a36dfdd737 100644 (file)
@@ -127,6 +127,7 @@ class hazptr_priv {
   hazptr_obj* head_{nullptr};
   hazptr_obj* tail_{nullptr};
   int rcount_{0};
+  bool active_{true};
 
  public:
   hazptr_priv();
@@ -608,6 +609,8 @@ inline hazptr_priv::hazptr_priv() {
 
 inline hazptr_priv::~hazptr_priv() {
   DEBUG_PRINT(this);
+  DCHECK(active_);
+  active_ = false;
   if (tail_) {
     pushAllToDomain();
   }
@@ -618,6 +621,10 @@ inline void hazptr_priv::push(hazptr_obj* obj) {
   if (tail_) {
     tail_->next_ = obj;
   } else {
+    if (!active_) {
+      default_hazptr_domain().objRetire(obj);
+      return;
+    }
     head_ = obj;
   }
   tail_ = obj;
index d40e9275fe75363419aa23abb2eb716c49831202..a669db8b69b1def884506a801fb43d5760199df9 100644 (file)
@@ -262,23 +262,32 @@ TEST_F(HazptrTest, VirtualTest) {
   }
 }
 
-TEST_F(HazptrTest, DestructionTest) {
-  hazptr_domain myDomain0;
+void destructionTest(hazptr_domain& domain) {
   struct Thing : public hazptr_obj_base<Thing> {
     Thing* next;
-    Thing(Thing* n) : next(n) {}
+    hazptr_domain* domain;
+    int val;
+    Thing(int v, Thing* n, hazptr_domain* d) : next(n), domain(d), val(v) {}
     ~Thing() {
-      DEBUG_PRINT("this: " << this << " next: " << next);
+      DEBUG_PRINT("this: " << this << " val: " << val << " next: " << next);
       if (next) {
-        next->retire();
+        next->retire(*domain);
       }
     }
   };
   Thing* last{nullptr};
   for (int i = 0; i < 2000; i++) {
-    last = new Thing(last);
+    last = new Thing(i, last, &domain);
+  }
+  last->retire(domain);
+}
+
+TEST_F(HazptrTest, DestructionTest) {
+  {
+    hazptr_domain myDomain0;
+    destructionTest(myDomain0);
   }
-  last->retire();
+  destructionTest(default_hazptr_domain());
 }
 
 TEST_F(HazptrTest, Move) {