From: Subodh Iyengar <subodh@fb.com>
Date: Mon, 21 Sep 2015 22:38:25 +0000 (-0700)
Subject: Fix potential delete behavior of guard
X-Git-Tag: deprecate-dynamic-initializer~385
X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=ae59a5e910d1091d6abc5d21708d4c3881e786b7;p=folly.git

Fix potential delete behavior of guard

Summary: There is a potential double free in destructor guard
if someone calls a method which takes a DG in the destructor
of the DG.

This is potential in the case when someone is holding onto
a DG while calling destroy() on the object.

Reviewed By: @djwatson

Differential Revision: D2463113
---

diff --git a/folly/io/async/DelayedDestruction.h b/folly/io/async/DelayedDestruction.h
index e7cf7d42..d65b0298 100644
--- a/folly/io/async/DelayedDestruction.h
+++ b/folly/io/async/DelayedDestruction.h
@@ -104,6 +104,7 @@ class DelayedDestruction : public DelayedDestructionBase {
       if (delayed && !destroyPending_) {
         return;
       }
+      destroyPending_ = false;
       delete this;
     };
   }
@@ -111,7 +112,7 @@ class DelayedDestruction : public DelayedDestructionBase {
  private:
   /**
    * destroyPending_ is set to true if destoy() is called while guardCount_ is
-   * non-zero.
+   * non-zero. It is set to false before the object is deleted.
    *
    * If destroyPending_ is true, the object will be destroyed the next time
    * guardCount_ drops to 0.
diff --git a/folly/io/async/test/DelayedDestructionTest.cpp b/folly/io/async/test/DelayedDestructionTest.cpp
new file mode 100644
index 00000000..e042ea3c
--- /dev/null
+++ b/folly/io/async/test/DelayedDestructionTest.cpp
@@ -0,0 +1,43 @@
+/*
+ * Copyright 2015 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include <folly/io/async/DelayedDestruction.h>
+
+#include <gtest/gtest.h>
+
+using namespace folly;
+
+class DeleteGuarder : public DelayedDestruction {
+
+  ~DeleteGuarder() {
+    doFoo();
+  }
+
+  void doFoo() {
+    DelayedDestructionBase::DestructorGuard dg(this);
+    LOG(INFO) << "foo";
+  }
+};
+
+TEST(DelayedDestructionTest, GuardOnDelete) {
+  auto dg = new DeleteGuarder();
+  dg->destroy();
+}
+
+TEST(DelayedDestructionTest, GuardOnDeleteWithPreGuard) {
+  auto dg = new DeleteGuarder();
+  DelayedDestructionBase::DestructorGuard guard(dg);
+  dg->destroy();
+}