[Synchronization] Fix a crash in WaitableEventWatcher

base/synchronization/waitable_event_watcher.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef BASE_SYNCHRONIZATION_WAITABLE_EVENT_WATCHER_H_
 #define BASE_SYNCHRONIZATION_WAITABLE_EVENT_WATCHER_H_
 
 #include "base/base_export.h"
 #include "base/macros.h"
-#include "base/sequence_checker.h"
 #include "build/build_config.h"
 
 #if defined(OS_WIN)
 #include "base/win/object_watcher.h"
+#include "base/win/scoped_handle.h"
 #else
 #include "base/callback.h"
+#include "base/sequence_checker.h"
 #include "base/synchronization/waitable_event.h"
 #endif
 
 namespace base {
 
 class Flag;
 class AsyncWaiter;
 class WaitableEvent;
 
 // This class provides a way to wait on a WaitableEvent asynchronously.
@@ skipping 22 matching lines @@
 // goes out of scope, the watcher_ will be destroyed, and there is no need to
 // worry about OnWaitableEventSignaled being called on a deleted MyClass
 // pointer.
 //
 // BEWARE: With automatically reset WaitableEvents, a signal may be lost if it
 // occurs just before a WaitableEventWatcher is deleted. There is currently no
 // safe way to stop watching an automatic reset WaitableEvent without possibly
 // missing a signal.
 //
 // NOTE: you /are/ allowed to delete the WaitableEvent while still waiting on
-// it with a Watcher. It will act as if the event was never signaled.
+// it with a Watcher. But pay attention: if the event was signaled and deleted

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

ISTM that all WaitableEventWatchers' EventCallbacks are only using the
WaitableEvent* passed to it for pointer comparison. This is good as it could be
deleted as this comment says. What do you think of changing it to a void*?

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

As for me it is better to remove that param at all.
I looked on several use cases and found only DCHECKs.

I chose to not modify WaitableEventWatcher interface in that CL (it isn't
related with the crash).
Can you create bug/task to do that?

[danakj, 2017/04/28 17:54:49]

I would like to do it together because the pointer was safe to use before but
now it's not, if you think its reasonable. Keeping the DCHECKs doesn't seem
harmful and they'd work with a void* too.

[atuchin, 2017/05/02 07:41:19]

Unfortunately, the pointer wasn't safe before.
As you can see I don't change the behaviour of WaitableEventWatcher.
I change only watching handle. 

And using void* here doesn't look a good solution for me because after deleting
such pointers are not unique
(a new WaitableEvent may have the same address). That why I would like to not
change it here.

+// right after, the callback may be called with deleted WaitableEvent pointer.
 
 class BASE_EXPORT WaitableEventWatcher
 #if defined(OS_WIN)
     : public win::ObjectWatcher::Delegate
 #endif
 {
  public:
   using EventCallback = OnceCallback<void(WaitableEvent*)>;
   WaitableEventWatcher();
 
@@ skipping 13 matching lines @@
   // Does nothing if no event is being watched, nor if the watch has completed.
   // The callback will *not* be called for the current watch after this
   // function returns. Since the callback runs on the same sequence as this
   // function, it cannot be called during this function either.
   void StopWatching();
 
  private:
 #if defined(OS_WIN)
   void OnObjectSignaled(HANDLE h) override;
 
+  win::ScopedHandle event_handle_;
   win::ObjectWatcher watcher_;
   EventCallback callback_;
   WaitableEvent* event_ = nullptr;
 #else
   // Instantiated in StartWatching(). Set before the callback runs. Reset in
   // StopWatching() or StartWatching().
   scoped_refptr<Flag> cancel_flag_;
 
   // Enqueued in the wait list of the watched WaitableEvent.
   AsyncWaiter* waiter_ = nullptr;
 
   // Kernel of the watched WaitableEvent.
   scoped_refptr<WaitableEvent::WaitableEventKernel> kernel_;
 
   // Ensures that StartWatching() and StopWatching() are called on the same
   // sequence.
   SequenceChecker sequence_checker_;
 #endif
 
   DISALLOW_COPY_AND_ASSIGN(WaitableEventWatcher);
 };
 
 }  // namespace base
 
 #endif  // BASE_SYNCHRONIZATION_WAITABLE_EVENT_WATCHER_H_