From eeb8fd7dc845693fdb4e61a6031c658f8592e05e Mon Sep 17 00:00:00 2001
From: "kenton@google.com"
 <kenton@google.com@630680e5-0e50-0410-840e-4b1c322b438d>
Date: Tue, 16 Feb 2010 21:59:09 +0000
Subject: Fix bug with permanent callbacks that delete themselves when run. 
 Patch from Evan Jones.

---
 CONTRIBUTORS.txt                             |  1 +
 src/google/protobuf/stubs/common.h           | 18 ++++++++++++------
 src/google/protobuf/stubs/common_unittest.cc | 12 ++++++++++++
 3 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt
index 7fe99ecb..7c1ac80d 100644
--- a/CONTRIBUTORS.txt
+++ b/CONTRIBUTORS.txt
@@ -83,5 +83,6 @@ Patch contributors:
     * Optimize Java serialization of strings so that UTF-8 encoding happens only
       once per string per serialization call.
     * Clean up some Java warnings.
+    * Fix bug with permanent callbacks that delete themselves when run.
   Michael Kucharski <m.kucharski@gmail.com>
     * Added CodedInputStream.getTotalBytesRead().
diff --git a/src/google/protobuf/stubs/common.h b/src/google/protobuf/stubs/common.h
index cdac37af..749e6e9d 100644
--- a/src/google/protobuf/stubs/common.h
+++ b/src/google/protobuf/stubs/common.h
@@ -843,8 +843,9 @@ class LIBPROTOBUF_EXPORT FunctionClosure0 : public Closure {
   ~FunctionClosure0();
 
   void Run() {
+    bool needs_delete = self_deleting_;  // read in case callback deletes
     function_();
-    if (self_deleting_) delete this;
+    if (needs_delete) delete this;
   }
 
  private:
@@ -862,8 +863,9 @@ class MethodClosure0 : public Closure {
   ~MethodClosure0() {}
 
   void Run() {
+    bool needs_delete = self_deleting_;  // read in case callback deletes
     (object_->*method_)();
-    if (self_deleting_) delete this;
+    if (needs_delete) delete this;
   }
 
  private:
@@ -884,8 +886,9 @@ class FunctionClosure1 : public Closure {
   ~FunctionClosure1() {}
 
   void Run() {
+    bool needs_delete = self_deleting_;  // read in case callback deletes
     function_(arg1_);
-    if (self_deleting_) delete this;
+    if (needs_delete) delete this;
   }
 
  private:
@@ -906,8 +909,9 @@ class MethodClosure1 : public Closure {
   ~MethodClosure1() {}
 
   void Run() {
+    bool needs_delete = self_deleting_;  // read in case callback deletes
     (object_->*method_)(arg1_);
-    if (self_deleting_) delete this;
+    if (needs_delete) delete this;
   }
 
  private:
@@ -929,8 +933,9 @@ class FunctionClosure2 : public Closure {
   ~FunctionClosure2() {}
 
   void Run() {
+    bool needs_delete = self_deleting_;  // read in case callback deletes
     function_(arg1_, arg2_);
-    if (self_deleting_) delete this;
+    if (needs_delete) delete this;
   }
 
  private:
@@ -952,8 +957,9 @@ class MethodClosure2 : public Closure {
   ~MethodClosure2() {}
 
   void Run() {
+    bool needs_delete = self_deleting_;  // read in case callback deletes
     (object_->*method_)(arg1_, arg2_);
-    if (self_deleting_) delete this;
+    if (needs_delete) delete this;
   }
 
  private:
diff --git a/src/google/protobuf/stubs/common_unittest.cc b/src/google/protobuf/stubs/common_unittest.cc
index 32c1d08e..4109a52c 100644
--- a/src/google/protobuf/stubs/common_unittest.cc
+++ b/src/google/protobuf/stubs/common_unittest.cc
@@ -182,11 +182,17 @@ class ClosureTest : public testing::Test {
     a_ = 0;
     b_ = NULL;
     c_.clear();
+    permanent_closure_ = NULL;
+  }
+
+  void DeleteClosureInCallback() {
+    delete permanent_closure_;
   }
 
   int a_;
   const char* b_;
   string c_;
+  Closure* permanent_closure_;
 
   static ClosureTest* current_instance_;
 };
@@ -340,6 +346,12 @@ TEST_F(ClosureTest, TestPermanentClosureMethod2) {
   delete closure;
 }
 
+TEST_F(ClosureTest, TestPermanentClosureDeleteInCallback) {
+  permanent_closure_ = NewPermanentCallback((ClosureTest*) this,
+      &ClosureTest::DeleteClosureInCallback);
+  permanent_closure_->Run();
+}
+
 }  // anonymous namespace
 }  // namespace protobuf
 }  // namespace google
-- 
cgit v1.2.3