Closed magreenblatt closed 2 years ago
Does the crash reproduce with a supported version? Does it reproduce with the CEF sample apps?
Original comment by Michael Merritt (Bitbucket: Michael Merritt).
Branch 4638 specifies macOS 10.11+ deployment, and this has been reproduced in several versions of macOS 11 and 12.
I have not tested this with the CEF sample apps, since it is intermittent.
However, given the crash point and associated code, it seems that the return value of ExtensionsBrowserClient::Get() should be checked before use, since if it is NULL then this exact crash would likely result.
Are there other possibilities here I am missing?
It’s likely something wrong with shutdown ordering, possibly in Chromium code. We ask whether it reproduces with supported CEF versions because you’re using an old version, and the issue may already be fixed in newer versions.
This issue was reported for an old/unsupported version. If the problem reproduces with supported CEF versions then we can re-open.
Original comment by Michael Merritt (Bitbucket: Michael Merritt).
I don’t know about the current status of any CEF-related issues that might be associated with helping to cause the crash, but the crash point itself is definitely in Chromium, and the code is still unchanged:
Also reproduces with 4664 on Windows.
The Windows stack trace is somewhat different:
[ 00 ] extensions::RendererStartupHelper::UntrackProcess(content::RenderProcessHost *)
[ 01 ] extensions::RendererStartupHelper::RenderProcessExited(content::RenderProcessHost *,content::ChildProcessTerminationInfo const &)
[ 02 ] content::RenderProcessHostImpl::Cleanup()
[ 03 ] content::RenderProcessHostImpl::DecrementKeepAliveRefCount(unsigned __int64)
[ 04 ] content::`anonymous namespace'::KeepAliveHandleImpl::~KeepAliveHandleImpl
[ 05 ] content::`anonymous namespace'::KeepAliveHandleImpl::~KeepAliveHandleImpl
[ 06 ] mojo::internal::BindingState<blink::mojom::KeepAliveHandle,mojo::UniquePtrImplRefTraits<blink::mojom::KeepAliveHandle,std::__1::default_delete<blink::mojom::KeepAliveHandle> > >::~BindingState
[ 07 ] mojo::ReceiverSetBase<mojo::Receiver<blink::mojom::KeepAliveHandle,mojo::UniquePtrImplRefTraits<blink::mojom::KeepAliveHandle,std::__1::default_delete<blink::mojom::KeepAliveHandle> > >,void>::ReceiverEntry::~ReceiverEntry
[ 08 ] autofill::AutofillWebDataBackendImpl::ResetUserData()
[ 09 ] std::__1::unique_ptr<mojo::ReceiverSetState::Entry,std::__1::default_delete<mojo::ReceiverSetState::Entry> >::reset(mojo::ReceiverSetState::Entry *)
[ 10 ] std::__1::__tree<std::__1::__value_type<unsigned long long,std::__1::unique_ptr<mojo::ReceiverSetState::Entry,std::__1::default_delete<mojo::ReceiverSetState::Entry> > >,std::__1::__map_value_compare<unsigned long long,std::__1::__value_type<unsigned long long,std::__1::unique_ptr<mojo::ReceiverSetState::Entry,std::__1::default_delete<mojo::ReceiverSetState::Entry> > >,std::__1::less<unsigned long long>,1>,std::__1::allocator<std::__1::__value_type<unsigned long long,std::__1::unique_ptr<mojo::ReceiverSetState::Entry,std::__1::default_delete<mojo::ReceiverSetState::Entry> > > > >::destroy
[ 11 ] mojo::ReceiverSetState::~ReceiverSetState()
[ 12 ] content::KeepAliveHandleFactory::Context::~Context
[ 13 ] base::internal::BindState<`lambda at ../../chrome/browser/resource_coordinator/tab_manager.cc:175:24',std::__1::unique_ptr<resource_coordinator::TabManager::ResourceCoordinatorSignalObserver,std::__1::default_delete<resource_coordinator::TabManager::ResourceCoordinatorSignalObserver> > >::Destroy
[ 14 ] base::internal::CallbackBase::~CallbackBase()
[ 15 ] base::sequence_manager::internal::TaskQueueImpl::UnregisterTaskQueue()
[ 16 ] base::sequence_manager::internal::SequenceManagerImpl::UnregisterTaskQueueImpl(std::__1::unique_ptr<base::sequence_manager::internal::TaskQueueImpl,std::__1::default_delete<base::sequence_manager::internal::TaskQueueImpl> >)
[ 17 ] base::sequence_manager::TaskQueue::ShutdownTaskQueue()
[ 18 ] content::BrowserTaskQueues::~BrowserTaskQueues()
[ 19 ] content::BrowserUIThreadScheduler::~BrowserUIThreadScheduler()
[ 20 ] std::__1::unique_ptr<content::BrowserUIThreadScheduler,std::__1::default_delete<content::BrowserUIThreadScheduler> >::reset(content::BrowserUIThreadScheduler *)
[ 21 ] content::BrowserTaskExecutor::UIThreadExecutor::~UIThreadExecutor()
[ 22 ] content::BrowserTaskExecutor::UIThreadExecutor::~UIThreadExecutor
[ 23 ] content::BrowserTaskExecutor::Shutdown()
[ 24 ] content::ContentMainRunnerImpl::Shutdown()
[ 25 ] CefMainRunner::FinalizeShutdown(base::OnceCallback<void >)
[ 26 ] CefMainRunner::Shutdown(base::OnceCallback<void >,base::OnceCallback<void >)
[ 27 ] CefContext::Shutdown()
[ 28 ] CefShutdown()
Another possibly related Windows crash from 4664 (`AlloyBrowserContext::extension_system()` is likely invalid):
[ 03 ] extensions::CefExtensionSystem::OnRequestContextDeleted(CefRequestContext *)
[ 04 ] AlloyBrowserContext::RemoveCefRequestContext(CefRequestContextImpl *)
[ 05 ] CefRequestContextImpl::~CefRequestContextImpl()
[ 06 ] [thunk]:CefRequestContextImpl::`vector deleting destructor'`vtordisp{4294967292,0}' (unsigned int)
[ 07 ] content::BrowserThread::DeleteOnThread<content::BrowserThread::UI>::Destruct<CefImageImpl>
[ 08 ] [thunk]:CefRequestContextImpl::Release`vtordisp{4294967292,0}' ()
[ 09 ] CefBrowserHostBase::~CefBrowserHostBase()
[ 10 ] [thunk]:AlloyBrowserHostImpl::`vector deleting destructor'`adjustor{16}' (unsigned int)
[ 11 ] [thunk]:CefBrowserHostBase::Release`vtordisp{4294967292,152}' ()
Testing with M99, it looks like AlloyBrowserMainParts::PostDestroyThreads
(which calls extensions::ExtensionsBrowserClient::Set(nullptr)
) is being called before ContentMainRunnerImpl::Shutdown
. Consequently ExtensionsBrowserClient::Get()
will return nullptr if RendererStartupHelper::UntrackProcess
is called via Shutdown
.
It doesn’t crash more frequently because often process_host
is nullptr in ~KeepAliveHandleImpl
(here), and consequently RenderProcessHostImpl::DecrementKeepAliveRefCount
is not called.
extensions_shell also calls ExtensionsBrowserClient::Set(nullptr)
from PostDestroyThreads
(same as CEF), whereas Chrome uses ~BrowserProcessImpl
and cast uses PostMainMessageLoopRun
alloy: Move ExtensionsBrowserClient ownership to BrowserProcess (fixes issue #3247)
Fixes a shutdown crash due to ExtensionsBrowserClient::Set(nullptr)
being
called too early. Some code that may occasionally be triggered via
content::ContentMainShutdown()
is expecting the extensions objects to still
be valid.
This new ownership pattern matches the code in chrome/.
→ <<cset 9e5e8208d8e4 (bb)>>
alloy: Move ExtensionsBrowserClient ownership to BrowserProcess (fixes issue #3247)
Fixes a shutdown crash due to ExtensionsBrowserClient::Set(nullptr)
being
called too early. Some code that may occasionally be triggered via
content::ContentMainShutdown()
is expecting the extensions objects to still
be valid.
This new ownership pattern matches the code in chrome/.
→ <<cset d87e3f41f66a (bb)>>
alloy: Move ExtensionsBrowserClient ownership to BrowserProcess (fixes issue #3247)
Fixes a shutdown crash due to ExtensionsBrowserClient::Set(nullptr)
being
called too early. Some code that may occasionally be triggered via
content::ContentMainShutdown()
is expecting the extensions objects to still
be valid.
This new ownership pattern matches the code in chrome/.
→ <<cset a00bca5aa590 (bb)>>
alloy: Move ExtensionsBrowserClient ownership to BrowserProcess (fixes issue #3247)
Fixes a shutdown crash due to ExtensionsBrowserClient::Set(nullptr)
being
called too early. Some code that may occasionally be triggered via
content::ContentMainShutdown()
is expecting the extensions objects to still
be valid.
This new ownership pattern matches the code in chrome/.
→ <<cset 9eb0954cde62 (bb)>>
Filed issue #3276 for the Windows crash which is still reproducing after this fix.
Original report by Michael Merritt (Bitbucket: Michael Merritt).
I am seeing an intermittent crash at shutdown on macOS (multiple versions) with branch 4638. The crash occurs during the CefShutdown() API. The relevant portion of the macOS crash log looks like this:
The relevant portion of the CEF code looks like this:
Line 207 is given as the failing line, and you can see that ExtensionsBrowserClient::Get() is called to retrieve a pointer that is immediately dereferenced without checking its value. My guess is that the pointer is intermittently NULL, which is what the crash report indicates. The fact that this pointer is used without first checking its value would seem to be a bug. Of course, the next question is why is the pointer NULL, and the answer to that may lead to another more-complicated issue.
Does this seem to be a correct analysis of this intermittent crash, or is there something else I should check?
Thanks in advance for any help you can provide.