Adds some CHECKs to MessagePumpMojo
The thread watcher appears to be kicking on when we're trying to
remove a handle. Not sure why. This converts some DCHECKs to CHECKs
and fixes handling of the deadline. I suspect none of this will help,
but it's worth a shot.
BUG=399769
TEST=none
R=darin@chromium.org
Review URL: https://codereview.chromium.org/454433003
Cr-Commit-Position: refs/heads/master@{#288949}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@288949 0039d316-1c4b-4281-b951-d872f2087c98
CrOS-Libchrome-Original-Commit: 95d70989f5653bf15314bca409e5cc33363558bf
diff --git a/base/message_loop/message_loop.cc b/base/message_loop/message_loop.cc
index c0d9b0e..c19978b 100644
--- a/base/message_loop/message_loop.cc
+++ b/base/message_loop/message_loop.cc
@@ -92,6 +92,8 @@
// time for every task that is added to the MessageLoop incoming queue.
bool AlwaysNotifyPump(MessageLoop::Type type) {
#if defined(OS_ANDROID)
+ // The Android UI message loop needs to get notified each time a task is added
+ // to the incoming queue.
return type == MessageLoop::TYPE_UI || type == MessageLoop::TYPE_JAVA;
#else
return false;
@@ -528,8 +530,6 @@
}
void MessageLoop::ScheduleWork(bool was_empty) {
- // The Android UI message loop needs to get notified each time
- // a task is added to the incoming queue.
if (was_empty || AlwaysNotifyPump(type_))
pump_->ScheduleWork();
}
diff --git a/mojo/common/data_pipe_utils.cc b/mojo/common/data_pipe_utils.cc
index cf9dc78..2a6b21e 100644
--- a/mojo/common/data_pipe_utils.cc
+++ b/mojo/common/data_pipe_utils.cc
@@ -11,7 +11,6 @@
#include "base/files/scoped_file.h"
#include "base/message_loop/message_loop.h"
#include "base/task_runner_util.h"
-#include "mojo/common/handle_watcher.h"
namespace mojo {
namespace common {
diff --git a/mojo/common/message_pump_mojo.cc b/mojo/common/message_pump_mojo.cc
index c7903f2..d4b4949 100644
--- a/mojo/common/message_pump_mojo.cc
+++ b/mojo/common/message_pump_mojo.cc
@@ -15,6 +15,19 @@
namespace mojo {
namespace common {
+namespace {
+
+MojoDeadline TimeTicksToMojoDeadline(base::TimeTicks time_ticks,
+ base::TimeTicks now) {
+ // The is_null() check matches that of HandleWatcher.
+ if (time_ticks.is_null())
+ return MOJO_DEADLINE_INDEFINITE;
+ const int64_t delta = (time_ticks - now).InMicroseconds();
+ return delta < 0 ? static_cast<MojoDeadline>(0) :
+ static_cast<MojoDeadline>(delta);
+}
+
+} // namespace
// State needed for one iteration of WaitMany. The first handle and flags
// corresponds to that of the control pipe.
@@ -52,10 +65,10 @@
const Handle& handle,
MojoHandleSignals wait_signals,
base::TimeTicks deadline) {
- DCHECK(handler);
+ CHECK(handler);
DCHECK(handle.is_valid());
// Assume it's an error if someone tries to reregister an existing handle.
- DCHECK_EQ(0u, handlers_.count(handle));
+ CHECK_EQ(0u, handlers_.count(handle));
Handler handler_data;
handler_data.handler = handler;
handler_data.wait_signals = wait_signals;
@@ -186,7 +199,7 @@
void MessagePumpMojo::RemoveFirstInvalidHandle(const WaitState& wait_state) {
// TODO(sky): deal with control pipe going bad.
- for (size_t i = 1; i < wait_state.handles.size(); ++i) {
+ for (size_t i = 0; i < wait_state.handles.size(); ++i) {
const MojoResult result =
Wait(wait_state.handles[i], wait_state.wait_signals[i], 0);
if (result == MOJO_RESULT_INVALID_ARGUMENT) {
@@ -196,9 +209,11 @@
CHECK(false);
} else if (result == MOJO_RESULT_FAILED_PRECONDITION ||
result == MOJO_RESULT_CANCELLED) {
+ CHECK_NE(i, 0u); // Indicates the control pipe went bad.
+
// Remove the handle first, this way if OnHandleError() tries to remove
// the handle our iterator isn't invalidated.
- DCHECK(handlers_.find(wait_state.handles[i]) != handlers_.end());
+ CHECK(handlers_.find(wait_state.handles[i]) != handlers_.end());
MessagePumpMojoHandler* handler =
handlers_[wait_state.handles[i]].handler;
handlers_.erase(wait_state.handles[i]);
@@ -209,9 +224,12 @@
}
void MessagePumpMojo::SignalControlPipe(const RunState& run_state) {
- // TODO(sky): deal with error?
- WriteMessageRaw(run_state.write_handle.get(), NULL, 0, NULL, 0,
- MOJO_WRITE_MESSAGE_FLAG_NONE);
+ const MojoResult result =
+ WriteMessageRaw(run_state.write_handle.get(), NULL, 0, NULL, 0,
+ MOJO_WRITE_MESSAGE_FLAG_NONE);
+ // If we can't write we likely won't wake up the thread and there is a strong
+ // chance we'll deadlock.
+ CHECK_EQ(MOJO_RESULT_OK, result);
}
MessagePumpMojo::WaitState MessagePumpMojo::GetWaitState(
@@ -230,16 +248,16 @@
MojoDeadline MessagePumpMojo::GetDeadlineForWait(
const RunState& run_state) const {
- base::TimeTicks min_time = run_state.delayed_work_time;
+ const base::TimeTicks now(internal::NowTicks());
+ MojoDeadline deadline = static_cast<MojoDeadline>(0);
+ if (!run_state.delayed_work_time.is_null())
+ deadline = TimeTicksToMojoDeadline(run_state.delayed_work_time, now);
for (HandleToHandler::const_iterator i = handlers_.begin();
i != handlers_.end(); ++i) {
- if (min_time.is_null() && i->second.deadline < min_time)
- min_time = i->second.deadline;
+ deadline = std::min(
+ TimeTicksToMojoDeadline(i->second.deadline, now), deadline);
}
- return min_time.is_null() ? MOJO_DEADLINE_INDEFINITE :
- std::max(static_cast<MojoDeadline>(0),
- static_cast<MojoDeadline>(
- (min_time - internal::NowTicks()).InMicroseconds()));
+ return deadline;
}
} // namespace common
diff --git a/mojo/common/message_pump_mojo.h b/mojo/common/message_pump_mojo.h
index 7513118..7b2c170 100644
--- a/mojo/common/message_pump_mojo.h
+++ b/mojo/common/message_pump_mojo.h
@@ -31,6 +31,7 @@
// Registers a MessagePumpMojoHandler for the specified handle. Only one
// handler can be registered for a specified handle.
+ // NOTE: a value of 0 for |deadline| indicates an indefinite timeout.
void AddHandler(MessagePumpMojoHandler* handler,
const Handle& handle,
MojoHandleSignals wait_signals,