From 4153ea34f326b4868d1233656a4774c3f213a781 Mon Sep 17 00:00:00 2001 From: sammc Date: Sun, 13 Mar 2016 23:16:09 -0700 Subject: Change mojo::Watcher to survive closing a handle and cancelling. Currently, if a handle is closed while a Watcher is watching it, its callback will notify it that the watch has been cancelled. However, if the Watcher is explicitly cancelled after the handle is closed but before the callback is called, it will attempt to call MojoCancelWatch() on the closed handle and DCHECK that it succeeds. This changes the DCHECK to also accept MOJO_ERROR_INVALID_ARGUMENT. Review URL: https://codereview.chromium.org/1795193005 Cr-Commit-Position: refs/heads/master@{#380936} --- mojo/public/cpp/system/tests/watcher_unittest.cc | 20 ++++++++++++++++++++ mojo/public/cpp/system/watcher.cc | 4 +++- 2 files changed, 23 insertions(+), 1 deletion(-) (limited to 'mojo') diff --git a/mojo/public/cpp/system/tests/watcher_unittest.cc b/mojo/public/cpp/system/tests/watcher_unittest.cc index ea78a56..f72f495 100644 --- a/mojo/public/cpp/system/tests/watcher_unittest.cc +++ b/mojo/public/cpp/system/tests/watcher_unittest.cc @@ -182,5 +182,25 @@ TEST_F(WatcherTest, NotifyOnMessageLoopDestruction) { b_watcher.Cancel(); } +TEST_F(WatcherTest, CloseAndCancel) { + ScopedMessagePipeHandle a, b; + CreateMessagePipe(nullptr, &a, &b); + + Watcher b_watcher; + EXPECT_EQ(MOJO_RESULT_OK, + b_watcher.Start(b.get(), MOJO_HANDLE_SIGNAL_READABLE, + OnReady([](MojoResult result) { FAIL(); }))); + EXPECT_TRUE(b_watcher.IsWatching()); + + // This should trigger the watcher above... + b.reset(); + // ...but the watcher is cancelled first. + b_watcher.Cancel(); + + EXPECT_FALSE(b_watcher.IsWatching()); + + base::RunLoop().RunUntilIdle(); +} + } // namespace } // namespace mojo diff --git a/mojo/public/cpp/system/watcher.cc b/mojo/public/cpp/system/watcher.cc index ad965ff..5723533 100644 --- a/mojo/public/cpp/system/watcher.cc +++ b/mojo/public/cpp/system/watcher.cc @@ -96,7 +96,9 @@ void Watcher::Cancel() { MojoResult result = MojoCancelWatch(handle_.value(), reinterpret_cast(this)); message_loop_observer_.reset(); - DCHECK_EQ(result, MOJO_RESULT_OK); + // |result| may be MOJO_RESULT_INVALID_ARGUMENT if |handle_| has closed, but + // OnHandleReady has not yet been called. + DCHECK(result == MOJO_RESULT_INVALID_ARGUMENT || result == MOJO_RESULT_OK); handle_.set_value(kInvalidHandleValue); callback_.Reset(); } -- cgit v1.1