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}
Review URL: https://codereview.chromium.org/583043005
Cr-Commit-Position: refs/heads/master@{#297338}
diff --git a/content/renderer/render_thread_impl.cc b/content/renderer/render_thread_impl.cc
index 3b2ff69..f60a36c 100644
--- a/content/renderer/render_thread_impl.cc
+++ b/content/renderer/render_thread_impl.cc
@@ -401,6 +401,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, "");
@@ -657,16 +664,24 @@
main_thread_compositor_task_runner_ = NULL;
- if (webkit_platform_support_)
- blink::shutdown();
-
- lazy_tls.Pointer()->Set(NULL);
+ gpu_channel_ = NULL;
// 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 (webkit_platform_support_)
+ 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 954c074..12d1ee0 100644
--- a/content/renderer/render_thread_impl.h
+++ b/content/renderer/render_thread_impl.h
@@ -120,6 +120,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);
virtual ~RenderThreadImpl();
virtual void Shutdown() OVERRIDE;
@@ -523,6 +525,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 dbe40235c..57142543 100644
--- a/content/renderer/renderer_main.cc
+++ b/content/renderer/renderer_main.cc
@@ -149,12 +149,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");
@@ -198,7 +199,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) {
@@ -214,7 +215,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;
