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 e7cf7d42c13992729fe7d80ca31e8e28673e29b8..d65b02982bd8482c2f1bbd2e5547241e9a864789 100644 (file)
--- 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 (file)
index 0000000..e042ea3
--- /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();
+}