Pending tasks in a message loop should be deleted before shutting down Blink
Currently Blink is shut down before all the pending tasks in the message loop are deleted. This is problematic in Oilpan because a destructor of the pending tasks can touch Oilpan objects. Because Oilpan is already detached from the renderer thread at that point, touching Oilpan objects in the destructor leads to a crash. (See the bug report for a concrete scenario.)
To prevent Blink objects from getting accessed after Blink is shut down, this CL deletes all pending tasks in a message loop before shutting down Blink.
BUG=411026
TEST=None. I cannot reproduce the crash.
Committed: https://crrev.com/fdd5612c20f777e1279efd7c1e99d82ed04afaaf
Cr-Commit-Position: refs/heads/master@{#296697}
Committed: https://crrev.com/16d32a9f7f6d1ebb639cacedb5156272a9fec764
Cr-Commit-Position: refs/heads/master@{#297338}
Review URL: https://codereview.chromium.org/583043005
Cr-Commit-Position: refs/heads/master@{#303557}
diff --git a/content/common/gpu/client/gpu_channel_host.cc b/content/common/gpu/client/gpu_channel_host.cc
index 77ef875..83bbad1 100644
--- a/content/common/gpu/client/gpu_channel_host.cc
+++ b/content/common/gpu/client/gpu_channel_host.cc
@@ -232,6 +232,13 @@
delete command_buffer;
}
+void GpuChannelHost::DestroyChannel() {
+ // channel_ must be destroyed on the main thread.
+ if (channel_.get() && !factory_->IsMainThread())
+ factory_->GetMainLoop()->DeleteSoon(FROM_HERE, channel_.release());
+ channel_.reset();
+}
+
void GpuChannelHost::AddRoute(
int route_id, base::WeakPtr<IPC::Listener> listener) {
DCHECK(MessageLoopProxy::current().get());
@@ -311,12 +318,9 @@
}
GpuChannelHost::~GpuChannelHost() {
- // channel_ must be destroyed on the main thread.
- if (!factory_->IsMainThread())
- factory_->GetMainLoop()->DeleteSoon(FROM_HERE, channel_.release());
+ DestroyChannel();
}
-
GpuChannelHost::MessageFilter::MessageFilter()
: lost_(false) {
}
diff --git a/content/common/gpu/client/gpu_channel_host.h b/content/common/gpu/client/gpu_channel_host.h
index ebbbdac3..9a734766 100644
--- a/content/common/gpu/client/gpu_channel_host.h
+++ b/content/common/gpu/client/gpu_channel_host.h
@@ -130,6 +130,9 @@
// Destroy a command buffer created by this channel.
void DestroyCommandBuffer(CommandBufferProxyImpl* command_buffer);
+ // Destroy this channel.
+ void DestroyChannel();
+
// Add a route for the current message loop.
void AddRoute(int route_id, base::WeakPtr<IPC::Listener> listener);
void RemoveRoute(int route_id);
diff --git a/content/renderer/render_thread_impl.cc b/content/renderer/render_thread_impl.cc
index 8566532..ffa82bd 100644
--- a/content/renderer/render_thread_impl.cc
+++ b/content/renderer/render_thread_impl.cc
@@ -408,6 +408,13 @@
Init();
}
+RenderThreadImpl::RenderThreadImpl(
+ scoped_ptr<base::MessageLoop> main_message_loop)
+ : ChildThread(Options(ShouldUseMojoChannel())),
+ main_message_loop_(main_message_loop.Pass()) {
+ Init();
+}
+
void RenderThreadImpl::Init() {
TRACE_EVENT_BEGIN_ETW("RenderThreadImpl::Init", 0, "");
@@ -665,16 +672,25 @@
main_thread_compositor_task_runner_ = NULL;
- if (blink_platform_impl_)
- blink::shutdown();
-
- lazy_tls.Pointer()->Set(NULL);
+ if (gpu_channel_.get())
+ gpu_channel_->DestroyChannel();
// TODO(port)
#if defined(OS_WIN)
// Clean up plugin channels before this thread goes away.
NPChannelBase::CleanupChannels();
#endif
+
+ // Shut down the message loop before shutting down Blink.
+ // This prevents a scenario where a pending task in the message loop accesses
+ // Blink objects after Blink shuts down.
+ // This must be at the very end of the shutdown sequence. You must not touch
+ // the message loop after this.
+ main_message_loop_.reset();
+ if (blink_platform_impl_)
+ blink::shutdown();
+
+ lazy_tls.Pointer()->Set(NULL);
}
bool RenderThreadImpl::Send(IPC::Message* msg) {
diff --git a/content/renderer/render_thread_impl.h b/content/renderer/render_thread_impl.h
index 831c982..28e59436 100644
--- a/content/renderer/render_thread_impl.h
+++ b/content/renderer/render_thread_impl.h
@@ -121,6 +121,8 @@
RenderThreadImpl();
// Constructor that's used when running in single process mode.
explicit RenderThreadImpl(const std::string& channel_name);
+ // Constructor that's used in RendererMain.
+ explicit RenderThreadImpl(scoped_ptr<base::MessageLoop> main_message_loop);
~RenderThreadImpl() override;
void Shutdown() override;
@@ -525,6 +527,11 @@
// GpuChannelHostFactory methods.
scoped_refptr<base::MessageLoopProxy> io_message_loop_proxy_;
+ // The message loop of the renderer main thread.
+ // This message loop should be destructed before the RenderThreadImpl
+ // shuts down Blink.
+ scoped_ptr<base::MessageLoop> main_message_loop_;
+
// A lazily initiated thread on which file operations are run.
scoped_ptr<base::Thread> file_thread_;
diff --git a/content/renderer/renderer_main.cc b/content/renderer/renderer_main.cc
index e82712c..5ef677a 100644
--- a/content/renderer/renderer_main.cc
+++ b/content/renderer/renderer_main.cc
@@ -148,12 +148,13 @@
// needs to be backed by a Foundation-level loop to process NSTimers. See
// http://crbug.com/306348#c24 for details.
scoped_ptr<base::MessagePump> pump(new base::MessagePumpNSRunLoop());
- base::MessageLoop main_message_loop(pump.Pass());
+ scoped_ptr<base::MessageLoop> main_message_loop(
+ new base::MessageLoop(pump.Pass()));
#else
// The main message loop of the renderer services doesn't have IO or UI tasks.
- base::MessageLoop main_message_loop;
+ scoped_ptr<base::MessageLoop> main_message_loop(new base::MessageLoop());
#endif
- main_message_loop.AddTaskObserver(&task_observer);
+ main_message_loop->AddTaskObserver(&task_observer);
base::PlatformThread::SetName("CrRendererMain");
@@ -202,7 +203,7 @@
// TODO(markus): Check if it is OK to unconditionally move this
// instruction down.
RenderProcessImpl render_process;
- new RenderThreadImpl();
+ new RenderThreadImpl(main_message_loop.Pass());
#endif
bool run_loop = true;
if (!no_sandbox) {
@@ -218,7 +219,7 @@
}
#if defined(OS_POSIX) && !defined(OS_MACOSX)
RenderProcessImpl render_process;
- new RenderThreadImpl();
+ new RenderThreadImpl(main_message_loop.Pass());
#endif
base::HighResolutionTimerManager hi_res_timer_manager;
