Skip to content

Commit

Permalink
Simplify SyncChannel message pumping event
Browse files Browse the repository at this point in the history
There's no need to use ref-counted or thread-local storage for
this. It can be a single leaky MojoEvent instance.

BUG=612500

Review-Url: https://codereview.chromium.org/2081963003
Cr-Commit-Position: refs/heads/master@{#401342}
  • Loading branch information
krockot authored and Commit bot committed Jun 22, 2016
1 parent fc041b4 commit 9981a62
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 40 deletions.
48 changes: 12 additions & 36 deletions ipc/ipc_sync_channel.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,48 +49,23 @@ void RunOnHandleReady(const base::Closure& callback, MojoResult result) {
callback.Run();
}

} // namespace

// A lazy thread-local Mojo Event which is always signaled. Used to wake up the
// sync waiter when a SyncMessage requires the MessageLoop to be pumped while
// waiting for a reply. This object is created lazily and ref-counted so it can
// be cleaned up when no longer in use.
class SyncChannel::PumpMessagesEvent
: public base::RefCountedThreadSafe<PumpMessagesEvent> {
class PumpMessagesEvent {
public:
static scoped_refptr<PumpMessagesEvent> current() {
scoped_refptr<PumpMessagesEvent> current = current_event_.Pointer()->Get();
if (!current) {
current = new PumpMessagesEvent;
current_event_.Pointer()->Set(current.get());
}
return current;
}
PumpMessagesEvent() { event_.Signal(); }
~PumpMessagesEvent() {}

const MojoEvent* event() const { return &event_; }

private:
friend class base::RefCountedThreadSafe<PumpMessagesEvent>;

PumpMessagesEvent() { event_.Signal(); }

~PumpMessagesEvent() {
DCHECK_EQ(current_event_.Pointer()->Get(), this);
current_event_.Pointer()->Set(nullptr);
}

MojoEvent event_;

static base::LazyInstance<base::ThreadLocalPointer<
SyncChannel::PumpMessagesEvent>> current_event_;

DISALLOW_COPY_AND_ASSIGN(PumpMessagesEvent);
};

base::LazyInstance<base::ThreadLocalPointer<SyncChannel::PumpMessagesEvent>>
SyncChannel::PumpMessagesEvent::current_event_ =
LAZY_INSTANCE_INITIALIZER;
base::LazyInstance<PumpMessagesEvent>::Leaky g_pump_messages_event =
LAZY_INSTANCE_INITIALIZER;

} // namespace

// When we're blocked in a Send(), we need to process incoming synchronous
// messages right away because it could be blocking our reply (either
Expand Down Expand Up @@ -503,7 +478,6 @@ SyncChannel::SyncChannel(
const scoped_refptr<base::SingleThreadTaskRunner>& ipc_task_runner,
WaitableEvent* shutdown_event)
: ChannelProxy(new SyncContext(listener, ipc_task_runner, shutdown_event)),
pump_messages_event_(PumpMessagesEvent::current()),
sync_handle_registry_(mojo::SyncHandleRegistry::current()) {
// The current (listener) thread must be distinct from the IPC thread, or else
// sending synchronous messages will deadlock.
Expand Down Expand Up @@ -559,9 +533,7 @@ bool SyncChannel::Send(Message* message) {
// Wait for reply, or for any other incoming synchronous messages.
// *this* might get deleted, so only call static functions at this point.
scoped_refptr<mojo::SyncHandleRegistry> registry = sync_handle_registry_;
scoped_refptr<PumpMessagesEvent> pump_messages_event = pump_messages_event_;
WaitForReply(registry.get(), context.get(),
pump_messages ? pump_messages_event->event() : nullptr);
WaitForReply(registry.get(), context.get(), pump_messages);

TRACE_EVENT_FLOW_END0(TRACE_DISABLED_BY_DEFAULT("ipc.flow"),
"SyncChannel::Send", context->GetSendDoneEvent());
Expand All @@ -571,9 +543,13 @@ bool SyncChannel::Send(Message* message) {

void SyncChannel::WaitForReply(mojo::SyncHandleRegistry* registry,
SyncContext* context,
const MojoEvent* pump_messages_event) {
bool pump_messages) {
context->DispatchMessages();

const MojoEvent* pump_messages_event = nullptr;
if (pump_messages)
pump_messages_event = g_pump_messages_event.Get().event();

while (true) {
bool dispatch = false;
bool send_done = false;
Expand Down
5 changes: 1 addition & 4 deletions ipc/ipc_sync_channel.h
Original file line number Diff line number Diff line change
Expand Up @@ -211,8 +211,6 @@ class IPC_EXPORT SyncChannel : public ChannelProxy {
};

private:
class PumpMessagesEvent;

SyncChannel(
Listener* listener,
const scoped_refptr<base::SingleThreadTaskRunner>& ipc_task_runner,
Expand All @@ -228,7 +226,7 @@ class IPC_EXPORT SyncChannel : public ChannelProxy {
// latter one also runs a nested message loop in the meantime.
static void WaitForReply(mojo::SyncHandleRegistry* registry,
SyncContext* context,
const MojoEvent* pump_messages_event);
bool pump_messages);

// Runs a nested message loop until a reply arrives, times out, or the process
// shuts down.
Expand All @@ -240,7 +238,6 @@ class IPC_EXPORT SyncChannel : public ChannelProxy {
// ChannelProxy overrides:
void OnChannelInit() override;

scoped_refptr<PumpMessagesEvent> pump_messages_event_;
scoped_refptr<mojo::SyncHandleRegistry> sync_handle_registry_;

// Used to signal events between the IPC and listener threads.
Expand Down

0 comments on commit 9981a62

Please sign in to comment.