Browse Source

fix: destroy url loader wrapper when JS env exits (#44574)

* fix: destroy url loader wrapper when JS env exits

* Revert "fix: destroy url loader wrapper when JS env exits"

This reverts commit 419151a98a16814ea63e9abc197c6ae27f48128c.

* Revert "Revert "fix: destroy url loader wrapper when JS env exits""

This reverts commit 4b401b03c62aca79498660f995825491ae52f179.

* fix: double free of JSChunkedDataPipeGetter

* fix: crash on process exit after stream completes
Robo 5 months ago
parent
commit
0d6743e79b

+ 12 - 7
shell/common/api/electron_api_url_loader.cc

@@ -383,13 +383,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);
@@ -719,14 +719,19 @@ void SimpleURLLoaderWrapper::OnDataReceived(std::string_view string_view,
 }
 
 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::OnResponseStarted(

+ 2 - 0
shell/common/api/electron_api_url_loader.h

@@ -21,6 +21,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-forward.h"
 
@@ -50,6 +51,7 @@ namespace electron::api {
 class SimpleURLLoaderWrapper final
     : public gin::Wrappable<SimpleURLLoaderWrapper>,
       public gin_helper::EventEmitterMixin<SimpleURLLoaderWrapper>,
+      public gin_helper::CleanedUpAtExit,
       private network::SimpleURLLoaderStreamConsumer,
       private network::mojom::URLLoaderNetworkServiceObserver {
  public: