Skip to content

Commit

Permalink
Fix race on ipc_task_runner().
Browse files Browse the repository at this point in the history
channel_->ClearIPCTaskRunner() clears ipc_task_runner_ on
ChildThreadImpl thread, but OnChannelOpened is called on another
thread.

This issues can be reproduced in synthetic environment:
1) Slowdown IO thread by applying patch:
diff --git a/ipc/ipc_sync_channel.cc b/ipc/ipc_sync_channel.cc
index c4f14dc..62adbd3 100644
--- a/ipc/ipc_sync_channel.cc
+++ b/ipc/ipc_sync_channel.cc
@@ -27,6 +27,8 @@
 #include "ipc/ipc_sync_message.h"
 #include "mojo/public/cpp/bindings/sync_event_watcher.h"

+#include "base/threading/platform_thread.h"
+
 using base::WaitableEvent;

 namespace IPC {
@@ -495,6 +497,8 @@ void SyncChannel::SyncContext::OnChannelError() {
 }

 void SyncChannel::SyncContext::OnChannelOpened() {
+  base::PlatformThread::Sleep(base::TimeDelta::FromSeconds(1));
+
   shutdown_watcher_.StartWatching(
       shutdown_event_,
       base::Bind(&SyncChannel::SyncContext::OnShutdownEventSignaled,

2) Run unit test, that have InProcessUtilityThreadHelper, for
example ExtensionServiceTest.ManagementPolicyRequiresEnable

3) The test will crash with stack:
[FATAL:scoped_refptr.h(208)] Check failed: ptr_.

Stack Trace:
  000aa409  logging::LogMessage::~LogMessage()
  00086191  scoped_refptr<base::SequencedTaskRunner>::operator->() const
  000d7d13  base::WaitableEventWatcher::StartWatching ...
  0001dce5  IPC::SyncChannel::SyncContext::OnChannelOpened()
  0009075d  base::OnceCallback<void ()>::Run() &&
  000991fb  base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*)
  000af463  base::internal::IncomingTaskQueue::RunTask(base::PendingTask*)
  000b1401  base::MessageLoop::RunTask(base::PendingTask*)
  000b16ed  base::MessageLoop::DeferOrRunPendingTask(base::PendingTask)
  000b17c3  base::MessageLoop::DoWork()
  000b3329  base::MessagePumpLibevent::Run(base::MessagePump::Delegate*)
  000b11bf  base::MessageLoop::Run(bool)
  000c9e01  base::RunLoop::Run()
  000eb387  base::Thread::Run(base::RunLoop*)
  000eb645  base::Thread::ThreadMain()
  000e6865  base::(anonymous namespace)::ThreadFunc(void*)

Change-Id: I7c61e1d967876dd414e19261528d5871f7fbbbf3
Reviewed-on: https://chromium-review.googlesource.com/922201
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537078}
  • Loading branch information
user32s authored and Commit Bot committed Feb 15, 2018
1 parent 3c8c5ba commit 25cb493
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion ipc/ipc_sync_channel.cc
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,7 @@ void SyncChannel::SyncContext::OnChannelOpened() {
shutdown_event_,
base::Bind(&SyncChannel::SyncContext::OnShutdownEventSignaled,
base::Unretained(this)),
ipc_task_runner());
base::SequencedTaskRunnerHandle::Get());
Context::OnChannelOpened();
}

Expand Down

0 comments on commit 25cb493

Please sign in to comment.