[Synchronization] Fix a crash in WaitableEventWatcher

base/synchronization/waitable_event_watcher_win.cc

Index: base/synchronization/waitable_event_watcher_win.cc
diff --git a/base/synchronization/waitable_event_watcher_win.cc b/base/synchronization/waitable_event_watcher_win.cc
index f3e114e3966149c610d5579ec69fddaf32555c5f..3d288a922facc81afb391bf397db71758b909c5e 100644
--- a/base/synchronization/waitable_event_watcher_win.cc
+++ b/base/synchronization/waitable_event_watcher_win.cc
@@ -13,25 +13,46 @@ namespace base {
 WaitableEventWatcher::WaitableEventWatcher() = default;
 
 WaitableEventWatcher::~WaitableEventWatcher() {
+  // Explicitly run StopWatching to avoid dependency on the members

[danakj, 2017/04/27 20:01:08]

nit: StopWatching()

[atuchin, 2017/04/28 06:44:29]

Done.

+  // destruction order (|event_handle_| must outlive working |watcher_|).

[danakj, 2017/04/27 20:01:08]

Normally I'd just put a comment in the header file on watcher_ saying something
like "Makes use of |event_handle_| which must outlive it." instead of writing
code in the destructor. Would that work?

[atuchin, 2017/04/28 06:44:30]

Ok, removed that code and added comments to header.

+  watcher_.StopWatching();
 }
 
 bool WaitableEventWatcher::StartWatching(WaitableEvent* event,
                                          EventCallback callback) {
+  DCHECK(event);
   callback_ = std::move(callback);
   event_ = event;
-  return watcher_.StartWatchingOnce(event->handle(), this);
+
+  // Duplicate and hold the event handle until a callback is returned or
+  // waiting is stopped.
+  HANDLE event_handle = nullptr;

[danakj, 2017/04/27 20:01:08]

Can we avoid having a member and local var with the same name other than then
underscore? Maybe duplicate_handle_ for the member?

[atuchin, 2017/04/28 06:44:29]

Ok, I renamed:
local event_handle -> handle
event_handle_ -> duplicated_event_handle_

+  if (!::DuplicateHandle(::GetCurrentProcess(),  // hSourceProcessHandle
+                         event->handle(),
+                         ::GetCurrentProcess(),  // hTargetProcessHandle
+                         &event_handle,
+                         0,      // dwDesiredAccess ignored due to SAME_ACCESS
+                         FALSE,  // !bInheritHandle
+                         DUPLICATE_SAME_ACCESS)) {
+    return false;
+  }
+  event_handle_.Set(event_handle);
+  return watcher_.StartWatchingOnce(event_handle, this);
 }
 
 void WaitableEventWatcher::StopWatching() {
   callback_.Reset();
   event_ = NULL;
   watcher_.StopWatching();
+  event_handle_.Close();
 }
 
 void WaitableEventWatcher::OnObjectSignaled(HANDLE h) {
+  DCHECK_EQ(event_handle_.Get(), h);
   WaitableEvent* event = event_;
   EventCallback callback = std::move(callback_);
   event_ = NULL;
+  event_handle_.Close();
   DCHECK(event);
 
   std::move(callback).Run(event);