[Synchronization] Fix a crash in WaitableEventWatcher

base/synchronization/waitable_event_watcher_unittest.cc

Index: base/synchronization/waitable_event_watcher_unittest.cc
diff --git a/base/synchronization/waitable_event_watcher_unittest.cc b/base/synchronization/waitable_event_watcher_unittest.cc
index 9df72892c3c3813adf8d1286c7bdf6dfefe48930..dac57198d73069b0620f3112d20260967c3abf0d 100644
--- a/base/synchronization/waitable_event_watcher_unittest.cc
+++ b/base/synchronization/waitable_event_watcher_unittest.cc
@@ -7,10 +7,12 @@
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/macros.h"
+#include "base/memory/ptr_util.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/threading/platform_thread.h"
+#include "base/threading/sequenced_task_runner_handle.h"
 #include "build/build_config.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -38,6 +40,7 @@ class DecrementCountContainer {
   explicit DecrementCountContainer(int* counter) : counter_(counter) {
   }
   void OnWaitableEventSignaled(WaitableEvent* object) {
+    // NOTE: |object| may be already deleted.
     --(*counter_);
   }
  private:
@@ -117,21 +120,61 @@ void RunTest_OutlivesMessageLoop(MessageLoop::Type message_loop_type) {
   }
 }
 
-void RunTest_DeleteUnder(MessageLoop::Type message_loop_type) {
+void RunTest_DeleteUnder(MessageLoop::Type message_loop_type,
+                         bool deferred_watcher_deletion) {
   // Delete the WaitableEvent out from under the Watcher. This is explictly
   // allowed by the interface.
+  // If |deferred_watcher_deletion| is false the watcher will be deleted right
+  // after deleting the event, which leads to immediately cancelling the wait
+  // operation.
+  // If |deferred_watcher_deletion| is true the watcher will be deleted
+  // in another callstack. That checks that the watcher can live with a deleted
+  // event when watching is still in progress.
 
   MessageLoop message_loop(message_loop_type);
 
   {
-    WaitableEventWatcher watcher;
+    auto watcher = base::MakeUnique<WaitableEventWatcher>();
+
+    auto event = base::MakeUnique<WaitableEvent>(
+        WaitableEvent::ResetPolicy::AUTOMATIC,
+        WaitableEvent::InitialState::NOT_SIGNALED);
+
+    watcher->StartWatching(event.get(), BindOnce(&QuitWhenSignaled));
+    event.reset();
+    if (deferred_watcher_deletion) {
+      base::SequencedTaskRunnerHandle::Get()->DeleteSoon(FROM_HERE,
+                                                         watcher.release());
+    }
+  }

[danakj, 2017/05/04 16:09:37]

What if you RunLoop().Run() here to let the watcher delete, and also post it
with a delay?

+}
+
+void RunTest_SignalAndDelete(MessageLoop::Type message_loop_type,
+                             bool deferred_watcher_deletion) {
+  // Signal and immediately delete the WaitableEvent out from under the Watcher.
+  // If |deferred_watcher_deletion| is false the watcher will be deleted right
+  // after deleting the event, which leads to immediately cancelling the wait
+  // operation.
+  // If |deferred_watcher_deletion| is true the watcher will be deleted
+  // in another callstack. That checks that the watcher can live with a deleted
+  // event when watching is still in progress.
 
-    WaitableEvent* event =
-        new WaitableEvent(WaitableEvent::ResetPolicy::AUTOMATIC,
-                          WaitableEvent::InitialState::NOT_SIGNALED);
+  MessageLoop message_loop(message_loop_type);
 
-    watcher.StartWatching(event, BindOnce(&QuitWhenSignaled));
-    delete event;
+  {
+    auto watcher = base::MakeUnique<WaitableEventWatcher>();
+
+    auto event = base::MakeUnique<WaitableEvent>(
+        WaitableEvent::ResetPolicy::AUTOMATIC,
+        WaitableEvent::InitialState::NOT_SIGNALED);
+
+    watcher->StartWatching(event.get(), BindOnce(&QuitWhenSignaled));
+    event->Signal();
+    event.reset();
+    if (deferred_watcher_deletion) {
+      base::SequencedTaskRunnerHandle::Get()->DeleteSoon(FROM_HERE,
+                                                         watcher.release());
+    }
   }
 }
 
@@ -165,7 +208,15 @@ TEST(WaitableEventWatcherTest, OutlivesMessageLoop) {
 
 TEST(WaitableEventWatcherTest, DeleteUnder) {
   for (int i = 0; i < kNumTestingMessageLoops; i++) {
-    RunTest_DeleteUnder(testing_message_loops[i]);
+    RunTest_DeleteUnder(testing_message_loops[i], false);
+    RunTest_DeleteUnder(testing_message_loops[i], true);
+  }
+}
+
+TEST(WaitableEventWatcherTest, SignalAndDelete) {
+  for (int i = 0; i < kNumTestingMessageLoops; i++) {
+    RunTest_SignalAndDelete(testing_message_loops[i], false);
+    RunTest_SignalAndDelete(testing_message_loops[i], true);
   }
 }