mirrors/electron
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix: shutdown crash in DownloadItem callback (#27342)

Browse source 
The call stack for one of our top crashes looks like this:

```
node::Abort (node_errors.cc:241)
node::Assert (node_errors.cc:256)
node::MakeCallback (callback.cc:226)
gin_helper::internal::CallMethodWithArgs (event_emitter_caller.cc:23)
gin_helper::EmitEvent<T> (event_emitter_caller.h:51)
gin_helper::EventEmitterMixin<T>::Emit<T> (event_emitter_mixin.h:81)
electron::api::DownloadItem::OnDownloadUpdated (electron_api_download_item.cc:115)
download::DownloadItemImpl::UpdateObservers (download_item_impl.cc:482)
content::DownloadManagerImpl::Shutdown (download_manager_impl.cc:508)
content::BrowserContext::~BrowserContext (browser_context.cc:476)
```

Full stack here: https://sentry.io/share/issue/9b030a0601b547188181b543c16ecda2/

During browser shutdown, the `DownloadManager` was being cleaned up
*after* the Node environment had already been destroyed. This caused the
`DownloadItem::OnDownloadUpdated` callback to crash when trying to emit
the JS `done` event.

To prevent this, we now manually shut down the `DownloadManager`
earlier. This is also mentioned in the comment on
`DownloadManager::Shutdown`:

```
// Shutdown the download manager. Content calls this when BrowserContext is
// being destructed. If the embedder needs this to be called earlier, it can
// call it. In that case, the delegate's Shutdown() method will only be called
// once.
```
This commit is contained in:
Biru Mohanathas 2021-01-20 21:30:08 +02:00 committed by GitHub
parent 7ddc756a08
commit 7f1e3ca3de
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
1 changed files with 10 additions and 0 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
shell/browser/electron_browser_main_parts.cc
View file

@ -520,6 +520,16 @@ void ElectronBrowserMainParts::PostMainMessageLoopRun() {
FreeAppDelegate(); FreeAppDelegate();
#endif #endif
// Shutdown the DownloadManager before destroying Node to prevent
// DownloadItem callbacks from crashing.
for (auto& iter : ElectronBrowserContext::browser_context_map()) {
auto* download_manager =
content::BrowserContext::GetDownloadManager(iter.second.get());
if (download_manager) {
download_manager->Shutdown();
}
}
// Make sure destruction callbacks are called before message loop is // Make sure destruction callbacks are called before message loop is
// destroyed, otherwise some objects that need to be deleted on IO thread // destroyed, otherwise some objects that need to be deleted on IO thread
// won't be freed. // won't be freed.