From 42c88eba7ccdec601539c7afb2b98a2fd83ad760 Mon Sep 17 00:00:00 2001 From: "trop[bot]" <37223003+trop[bot]@users.noreply.github.com> Date: Tue, 19 Nov 2024 22:08:28 +0100 Subject: [PATCH] fix: destroy url loader wrapper when JS env exits (#44730) * fix: destroy url loader wrapper when JS env exits Co-authored-by: deepak1556 * Revert "fix: destroy url loader wrapper when JS env exits" This reverts commit 419151a98a16814ea63e9abc197c6ae27f48128c. Co-authored-by: deepak1556 * Revert "Revert "fix: destroy url loader wrapper when JS env exits"" This reverts commit 4b401b03c62aca79498660f995825491ae52f179. Co-authored-by: deepak1556 * fix: double free of JSChunkedDataPipeGetter Co-authored-by: deepak1556 * fix: crash on process exit after stream completes Co-authored-by: deepak1556 --------- Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com> Co-authored-by: deepak1556 --- shell/common/api/electron_api_url_loader.cc | 19 ++++++++++++------- shell/common/api/electron_api_url_loader.h | 2 ++ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/shell/common/api/electron_api_url_loader.cc b/shell/common/api/electron_api_url_loader.cc index 8f91198d6c..135a386587 100644 --- a/shell/common/api/electron_api_url_loader.cc +++ b/shell/common/api/electron_api_url_loader.cc @@ -379,13 +379,13 @@ void SimpleURLLoaderWrapper::Start() { loader_->SetAllowHttpErrorResults(true); loader_->SetURLLoaderFactoryOptions(request_options_); loader_->SetOnResponseStartedCallback(base::BindOnce( - &SimpleURLLoaderWrapper::OnResponseStarted, base::Unretained(this))); + &SimpleURLLoaderWrapper::OnResponseStarted, weak_factory_.GetWeakPtr())); loader_->SetOnRedirectCallback(base::BindRepeating( - &SimpleURLLoaderWrapper::OnRedirect, base::Unretained(this))); + &SimpleURLLoaderWrapper::OnRedirect, weak_factory_.GetWeakPtr())); loader_->SetOnUploadProgressCallback(base::BindRepeating( - &SimpleURLLoaderWrapper::OnUploadProgress, base::Unretained(this))); + &SimpleURLLoaderWrapper::OnUploadProgress, weak_factory_.GetWeakPtr())); loader_->SetOnDownloadProgressCallback(base::BindRepeating( - &SimpleURLLoaderWrapper::OnDownloadProgress, base::Unretained(this))); + &SimpleURLLoaderWrapper::OnDownloadProgress, weak_factory_.GetWeakPtr())); url_loader_factory_ = GetURLLoaderFactoryForURL(request_ref->url); loader_->DownloadAsStream(url_loader_factory_.get(), this); @@ -716,14 +716,19 @@ void SimpleURLLoaderWrapper::OnDataReceived(std::string_view string_piece, } void SimpleURLLoaderWrapper::OnComplete(bool success) { + auto self = weak_factory_.GetWeakPtr(); if (success) { Emit("complete"); } else { Emit("error", net::ErrorToString(loader_->NetError())); } - loader_.reset(); - pinned_wrapper_.Reset(); - pinned_chunk_pipe_getter_.Reset(); + // If users initiate process shutdown when the event is emitted, then + // we would perform cleanup of the wrapper and we should bail out below. + if (self) { + loader_.reset(); + pinned_wrapper_.Reset(); + pinned_chunk_pipe_getter_.Reset(); + } } void SimpleURLLoaderWrapper::OnRetry(base::OnceClosure start_retry) {} diff --git a/shell/common/api/electron_api_url_loader.h b/shell/common/api/electron_api_url_loader.h index eae61f6495..afc9f11413 100644 --- a/shell/common/api/electron_api_url_loader.h +++ b/shell/common/api/electron_api_url_loader.h @@ -22,6 +22,7 @@ #include "services/network/public/mojom/url_loader_network_service_observer.mojom.h" #include "services/network/public/mojom/url_response_head.mojom.h" #include "shell/browser/event_emitter_mixin.h" +#include "shell/common/gin_helper/cleaned_up_at_exit.h" #include "url/gurl.h" #include "v8/include/v8.h" @@ -47,6 +48,7 @@ namespace electron::api { class SimpleURLLoaderWrapper final : public gin::Wrappable, public gin_helper::EventEmitterMixin, + public gin_helper::CleanedUpAtExit, private network::SimpleURLLoaderStreamConsumer, private network::mojom::URLLoaderNetworkServiceObserver { public: