Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix: Shutdown crash in DownloadItem callback (#27419)
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. ``` Co-authored-by: Biru Mohanathas <birunthan@mohanathas.com>
- Loading branch information