Fixing race between ~AsyncInvoker and ~AsyncClosure, using ref-counting.

webrtc/rtc_base/thread_unittest.cc

Index: webrtc/rtc_base/thread_unittest.cc
diff --git a/webrtc/rtc_base/thread_unittest.cc b/webrtc/rtc_base/thread_unittest.cc
index a8c20d1b772699df4a782cc86fc0e5170c1bd404..e7701e84b38e4ea54fb76099c336bd7b21b24745 100644
--- a/webrtc/rtc_base/thread_unittest.cc
+++ b/webrtc/rtc_base/thread_unittest.cc
@@ -458,19 +458,20 @@ TEST_F(AsyncInvokeTest, KillInvokerDuringExecute) {
   thread->Start();
   volatile bool invoker_destroyed = false;
   {
+    auto functor = [&functor_started, &functor_continue, &functor_finished,
+                    &invoker_destroyed] {
+      functor_started.Set();
+      functor_continue.Wait(Event::kForever);
+      rtc::Thread::Current()->SleepMs(kWaitTimeout);
+      EXPECT_FALSE(invoker_destroyed);
+      functor_finished.Set();
+    };
     AsyncInvoker invoker;
-    invoker.AsyncInvoke<void>(RTC_FROM_HERE, thread.get(),
-                              [&functor_started, &functor_continue,
-                               &functor_finished, &invoker_destroyed] {
-                                functor_started.Set();
-                                functor_continue.Wait(Event::kForever);
-                                rtc::Thread::Current()->SleepMs(kWaitTimeout);
-                                EXPECT_FALSE(invoker_destroyed);
-                                functor_finished.Set();
-                              });
+    invoker.AsyncInvoke<void>(RTC_FROM_HERE, thread.get(), functor);
     functor_started.Wait(Event::kForever);
 
-    // Allow the functor to continue and immediately destroy the invoker.
+    // Destroy the invoker while the functor is still executing (doing
+    // SleepMs).
     functor_continue.Set();
   }
 
@@ -481,6 +482,37 @@ TEST_F(AsyncInvokeTest, KillInvokerDuringExecute) {
   functor_finished.Wait(Event::kForever);
 }
 
+// Variant of the above test where the async-invoked task calls AsyncInvoke
+// *again*, for the thread on which the AsyncInvoker is currently being
+// destroyed. This shouldn't deadlock or crash; this second invocation should
+// just be ignored.
+TEST_F(AsyncInvokeTest, KillInvokerDuringExecuteWithReentrantInvoke) {
+  Event functor_started(false, false);
+  // Flag used to verify that the recursively invoked task never actually runs.
+  bool reentrant_functor_run = false;
+
+  Thread* main = Thread::Current();
+  Thread thread;
+  thread.Start();
+  {
+    AsyncInvoker invoker;
+    auto reentrant_functor = [&reentrant_functor_run] {
+      reentrant_functor_run = true;
+    };
+    auto functor = [&functor_started, &invoker, main, reentrant_functor] {
+      functor_started.Set();
+      Thread::Current()->SleepMs(kWaitTimeout);
+      invoker.AsyncInvoke<void>(RTC_FROM_HERE, main, reentrant_functor);
+    };
+    // This queues a task on |thread| to sleep for |kWaitTimeout| then queue a
+    // task on |main|. But this second queued task should never run, since the
+    // destructor will be entered before it's even invoked.
+    invoker.AsyncInvoke<void>(RTC_FROM_HERE, &thread, functor);
+    functor_started.Wait(Event::kForever);
+  }
+  EXPECT_FALSE(reentrant_functor_run);
+}
+
 TEST_F(AsyncInvokeTest, Flush) {
   AsyncInvoker invoker;
   AtomicBool flag1;