diff --git a/services/proxy_resolver/proxy_resolver_v8.cc b/services/proxy_resolver/proxy_resolver_v8.cc index cd259358aaa49..06cde4125ec09 100644 --- a/services/proxy_resolver/proxy_resolver_v8.cc +++ b/services/proxy_resolver/proxy_resolver_v8.cc @@ -453,6 +453,19 @@ class ProxyResolverV8::Context { v8::ContextDependants::kNoDependants); } + // ContextDisposedNotification only prunes dirty FinalizationRegistries; it + // does not cancel queued ResolveAsyncWaiterPromisesTask foreground tasks + // or flush the isolate's shared default MicrotaskQueue, both of which can + // still strongly root this v8::Context (and the Functions whose + // v8::External data points at |this|). Neutralize the indirection while + // holding the v8::Locker so any later callback observes a null Context* + // instead of a dangling one. The tiny ContextHolder is intentionally + // leaked because the v8::External may outlive us. + if (holder_) { + holder_->context = nullptr; + holder_.release(); + } + v8_this_.Reset(); v8_context_.Reset(); } @@ -523,9 +536,11 @@ class ProxyResolverV8::Context { v8::Isolate::Scope isolate_scope(isolate_); v8::HandleScope scope(isolate_); - v8_this_.Reset( - isolate_, - v8::External::New(isolate_, this, gin::kProxyResolverV8ContextTag)); + holder_ = std::make_unique(); + holder_->context = this; + v8_this_.Reset(isolate_, + v8::External::New(isolate_, holder_.get(), + gin::kProxyResolverV8ContextTag)); v8::Local v8_this = v8::Local::New(isolate_, v8_this_); @@ -693,9 +708,10 @@ class ProxyResolverV8::Context { // V8 callback for when "alert()" is invoked by the PAC script. static void AlertCallback(const v8::FunctionCallbackInfo& args) { - Context* context = - static_cast(v8::External::Cast(*args.Data()) - ->Value(gin::kProxyResolverV8ContextTag)); + Context* context = ContextFromArgs(args); + if (!context) { + return; + } // Like firefox we assume "undefined" if no argument was specified, and // disregard any arguments beyond the first. @@ -742,9 +758,10 @@ class ProxyResolverV8::Context { static void DnsResolveCallbackHelper( const v8::FunctionCallbackInfo& args, net::ProxyResolveDnsOperation op) { - Context* context = - static_cast(v8::External::Cast(*args.Data()) - ->Value(gin::kProxyResolverV8ContextTag)); + Context* context = ContextFromArgs(args); + if (!context) { + return; + } std::string hostname; @@ -860,9 +877,29 @@ class ProxyResolverV8::Context { args.GetReturnValue().Set(IsPlainHostName(hostname_utf8)); } + // Indirection for the v8::External bound to the JS callback Functions. The + // v8::External's value is immutable and the Functions can outlive |this| + // (e.g. via Atomics.waitAsync reactions queued in the shared isolate + // MicrotaskQueue), so callbacks must go through a holder that ~Context() + // nulls under the v8::Locker. + struct ContextHolder { + // Nulled in ~Context() before the holder is (intentionally) leaked, so + // the raw_ptr never dangles. + raw_ptr context = nullptr; + }; + + static Context* ContextFromArgs( + const v8::FunctionCallbackInfo& args) { + auto* holder = static_cast( + v8::External::Cast(*args.Data()) + ->Value(gin::kProxyResolverV8ContextTag)); + return holder ? holder->context : nullptr; + } + mutable base::Lock lock_; raw_ptr js_bindings_ = nullptr; raw_ptr isolate_; + std::unique_ptr holder_; v8::Persistent v8_this_; v8::Persistent v8_context_; };