Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(3024)

Unified Diff: base/synchronization/waitable_event_watcher_unittest.cc

Issue 2839213002: [Synchronization] Fix a crash in WaitableEventWatcher (Closed)
Patch Set: Fix grammar in comments Created 3 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « base/synchronization/waitable_event_watcher.h ('k') | base/synchronization/waitable_event_watcher_win.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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..713ecb8e544de5add714b02f45a84852d2b2f610 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,7 +120,8 @@ 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 delay_after_delete) {
// Delete the WaitableEvent out from under the Watcher. This is explictly
// allowed by the interface.
@@ -126,15 +130,55 @@ void RunTest_DeleteUnder(MessageLoop::Type message_loop_type) {
{
WaitableEventWatcher watcher;
- WaitableEvent* event =
- new WaitableEvent(WaitableEvent::ResetPolicy::AUTOMATIC,
- WaitableEvent::InitialState::NOT_SIGNALED);
+ auto* event = new WaitableEvent(WaitableEvent::ResetPolicy::AUTOMATIC,
+ WaitableEvent::InitialState::NOT_SIGNALED);
watcher.StartWatching(event, BindOnce(&QuitWhenSignaled));
+
+ if (delay_after_delete) {
+ // On Windows that sleep() improves the chance to catch some problems.
+ // It postpones the dtor |watcher| (which immediately cancel the waiting)
+ // and gives some time to run to a created background thread.
+ // Unfortunately, that thread is under OS control and we can't
+ // manipulate it directly.
+ base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(30));
+ }
+
delete event;
}
}
+void RunTest_SignalAndDelete(MessageLoop::Type message_loop_type,
+ bool delay_after_delete) {
+ // Signal and immediately delete the WaitableEvent out from under the Watcher.
+
+ MessageLoop message_loop(message_loop_type);
+
+ {
+ WaitableEventWatcher watcher;
+
+ auto event = base::MakeUnique<WaitableEvent>(
+ WaitableEvent::ResetPolicy::AUTOMATIC,
+ WaitableEvent::InitialState::NOT_SIGNALED);
+
+ watcher.StartWatching(event.get(), BindOnce(&QuitWhenSignaled));
+ event->Signal();
+ event.reset();
+
+ if (delay_after_delete) {
+ // On Windows that sleep() improves the chance to catch some problems.
+ // It postpones the dtor |watcher| (which immediately cancel the waiting)
+ // and gives some time to run to a created background thread.
+ // Unfortunately, that thread is under OS control and we can't
+ // manipulate it directly.
+ base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(30));
+ }
+
+ // Wait for the watcher callback.
+ RunLoop().Run();
+ }
+}
+
} // namespace
//-----------------------------------------------------------------------------
@@ -165,7 +209,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);
}
}
« no previous file with comments | « base/synchronization/waitable_event_watcher.h ('k') | base/synchronization/waitable_event_watcher_win.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698