Commit bc961340 authored by Aaron Colwell's avatar Aaron Colwell Committed by Commit Bot

Prevent creation of RenderProcessHostImpls that reference a BrowserContext that is shutting down.

This change prevents code from creating RenderProcessHostImpl objects
that will reference a BrowserContext that is in the process of shutting
down. Since RPHI's reference to a BrowserContext is a raw pointer it is
easy to accidentally introduce a potential use after free if an RPHI is
created as the BrowserContext is shutting down. The goal of the CHECK
is to help detect code that might be trying to create RPHIs at
inappropriate times.

Also added logic to avoid creating a spare RPHI for BrowserContexts that
are in the process of shutting down. This should help mitigate crashes
that we are seeing where destruction of the spare appears to be
referencing a BrowserContext that has already been destroyed. Added a
DWOC on this path to help us detect potentially problematic callers.

Bug: 1099998 1038844
Change-Id: I9a02e6f9cf19394a990afe3ca41f7d370ff3e2c4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2317827Reviewed-by: default avatarNasko Oskov <nasko@chromium.org>
Commit-Queue: Aaron Colwell <acolwell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791377}
parent 4308c4ca
......@@ -519,6 +519,18 @@ class SpareRenderProcessHostManager : public RenderProcessHostObserver {
CleanupSpareRenderProcessHost();
// Don't create a spare renderer for a BrowserContext that is in the
// process of shutting down.
if (browser_context->ShutdownStarted()) {
// Create a crash dump to help us assess what scenarios trigger this
// path to be taken.
// TODO(acolwell): Remove this call once are confident we've eliminated
// any problematic callers.
base::debug::DumpWithoutCrashing();
return;
}
// Don't create a spare renderer if we're using --single-process or if we've
// got too many processes. See also ShouldTryToUseExistingProcessHost in
// this file.
......@@ -1572,6 +1584,7 @@ RenderProcessHostImpl::RenderProcessHostImpl(
instance_weak_factory_(base::in_place, this),
frame_sink_provider_(id_),
shutdown_exit_code_(-1) {
CHECK(!browser_context->ShutdownStarted());
TRACE_EVENT2("shutdown", "RenderProcessHostImpl", "render_process_host", this,
"id", GetID());
TRACE_EVENT_NESTABLE_ASYNC_BEGIN2("shutdown", "Browser.RenderProcessHostImpl",
......
......@@ -229,6 +229,11 @@ class CONTENT_EXPORT BrowserContext : public base::SupportsUserData {
// StoragePartition can have time to do necessary cleanups on IO thread.
void ShutdownStoragePartitions();
// Returns true if shutdown has been initiated via a
// NotifyWillBeDestroyed() call. This is a signal that the object will be
// destroyed soon and no new references to this object should be created.
bool ShutdownStarted() { return was_notify_will_be_destroyed_called_; }
#if !defined(OS_ANDROID)
// Creates a delegate to initialize a HostZoomMap and persist its information.
// This is called during creation of each StoragePartition.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment