From 6a087f634d4fb5b23e16ac0586de1736258f0ad3 Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Tue, 11 Aug 2026 18:36:17 -0300 Subject: [PATCH] fix: delete self-owned URL wrappers at isolate teardown URLImpl, URLSearchParamsImpl and URLPatternImpl free themselves from a SetWeak(kParameter) finalizer, but weak callbacks never fire at isolate disposal, so every instance still alive when a Runtime is destroyed leaked its ada state (and, for URLPattern, its compiled RegExp globals). Consolidate the three copy-pasted weak-handle/finalizer blocks into an IsolateTracked base that also registers each instance in a per-isolate robin_hood set; ~Runtime drains the survivors right after ObjectManager::DisposeAllRegistered, while the isolate is still alive under the Locker so destructors may reset their v8::Global handles. --- NativeScript/runtime/Caches.h | 6 +++ NativeScript/runtime/IsolateTracked.h | 57 ++++++++++++++++++++++ NativeScript/runtime/Runtime.mm | 2 + NativeScript/runtime/URLImpl.h | 17 +------ NativeScript/runtime/URLPatternImpl.h | 17 +------ NativeScript/runtime/URLSearchParamsImpl.h | 17 +------ 6 files changed, 71 insertions(+), 45 deletions(-) create mode 100644 NativeScript/runtime/IsolateTracked.h diff --git a/NativeScript/runtime/Caches.h b/NativeScript/runtime/Caches.h index 68c7f1b7..2e90c79d 100644 --- a/NativeScript/runtime/Caches.h +++ b/NativeScript/runtime/Caches.h @@ -14,6 +14,7 @@ namespace tns { struct StructInfo; struct ObjectWeakCallbackState; class PromiseRejectionTracker; +class IsolateTracked; struct pair_hash { template @@ -129,6 +130,11 @@ class Caches { std::shared_ptr>> PointerInstances; + // Live IsolateTracked instances (URL, URLSearchParams, URLPattern). Their + // weak-callback finalizers never fire at isolate disposal, so teardown + // sweeps this set to delete whatever GC didn't get to. + robin_hood::unordered_set TrackedInstances; + std::function( v8::Local, const BaseClassMeta*, KnownUnknownClassPair, const std::vector&)> diff --git a/NativeScript/runtime/IsolateTracked.h b/NativeScript/runtime/IsolateTracked.h new file mode 100644 index 00000000..9767ac84 --- /dev/null +++ b/NativeScript/runtime/IsolateTracked.h @@ -0,0 +1,57 @@ +#pragma once + +#include "Caches.h" +#include "Common.h" + +namespace tns { + +// Base for self-owned C++ objects whose lifetime is bound to a single JS +// object through a weak handle. Instances die in exactly two places: the GC +// finalizer, or SweepAll at isolate teardown — weak callbacks never fire at +// isolate disposal, so anything still registered there must be deleted +// explicitly or it leaks. Never delete a bound instance directly; both +// deletion paths own the registry bookkeeping. +class IsolateTracked { + public: + virtual ~IsolateTracked() = default; + + void BindFinalizer(v8::Isolate* isolate, + const v8::Local& object) { + v8::HandleScope scopedHandle(isolate); + weakHandle_.Reset(isolate, object); + weakHandle_.SetWeak(this, Finalizer, v8::WeakCallbackType::kParameter); + std::shared_ptr cache = Caches::Get(isolate); + if (cache != nullptr) { + cache->TrackedInstances.insert(this); + } + } + + // Runs in ~Runtime under the Locker, while the isolate is still alive — + // destructors may Reset v8::Global handles but must not create new ones. + static void SweepAll(v8::Isolate* isolate) { + std::shared_ptr cache = Caches::Get(isolate); + if (cache == nullptr) { + return; + } + // Detach the set first so destructors can't mutate it mid-walk. + auto survivors = std::move(cache->TrackedInstances); + cache->TrackedInstances.clear(); + for (IsolateTracked* instance : survivors) { + delete instance; + } + } + + private: + static void Finalizer(const v8::WeakCallbackInfo& data) { + IsolateTracked* self = data.GetParameter(); + std::shared_ptr cache = Caches::Get(data.GetIsolate()); + if (cache != nullptr) { + cache->TrackedInstances.erase(self); + } + delete self; + } + + v8::Global weakHandle_; +}; + +} // namespace tns diff --git a/NativeScript/runtime/Runtime.mm b/NativeScript/runtime/Runtime.mm index 6ff6c072..4e1f219c 100644 --- a/NativeScript/runtime/Runtime.mm +++ b/NativeScript/runtime/Runtime.mm @@ -11,6 +11,7 @@ #include "Helpers.h" #include "InlineFunctions.h" #include "Interop.h" +#include "IsolateTracked.h" #include "NativeScriptException.h" #include "ObjectManager.h" #include "Performance.h" @@ -232,6 +233,7 @@ void DisposeIsolateWhenPossible(Isolate* isolate) { g_moduleRegistry.clear(); ObjectManager::DisposeAllRegistered(isolate_); + IsolateTracked::SweepAll(isolate_); if (IsRuntimeWorker()) { auto currentWorker = diff --git a/NativeScript/runtime/URLImpl.h b/NativeScript/runtime/URLImpl.h index de68a152..4e4e6ec8 100644 --- a/NativeScript/runtime/URLImpl.h +++ b/NativeScript/runtime/URLImpl.h @@ -7,11 +7,12 @@ #include #include "Common.h" +#include "IsolateTracked.h" #include "ada/ada.h" using namespace ada; namespace tns { -class URLImpl { +class URLImpl : public IsolateTracked { public: URLImpl(url_aggregator url); @@ -99,21 +100,7 @@ class URLImpl { static void CanParse(const v8::FunctionCallbackInfo& args); - void BindFinalizer(v8::Isolate* isolate, - const v8::Local& object) { - v8::HandleScope scopedHandle(isolate); - weakHandle_.Reset(isolate, object); - weakHandle_.SetWeak(this, Finalizer, v8::WeakCallbackType::kParameter); - } - - static void Finalizer(const v8::WeakCallbackInfo& data) { - auto* pThis = data.GetParameter(); - pThis->weakHandle_.Reset(); - delete pThis; - } - private: url_aggregator url_; - v8::Global weakHandle_; }; } // namespace tns diff --git a/NativeScript/runtime/URLPatternImpl.h b/NativeScript/runtime/URLPatternImpl.h index c9110544..b9f8ff37 100644 --- a/NativeScript/runtime/URLPatternImpl.h +++ b/NativeScript/runtime/URLPatternImpl.h @@ -5,6 +5,7 @@ #include "Common.h" #include "Helpers.h" +#include "IsolateTracked.h" #include "ada/ada.h" using namespace ada; namespace tns { @@ -22,7 +23,7 @@ class v8_regex_provider { static bool regex_match(std::string_view input, const regex_type& pattern); }; -class URLPatternImpl { +class URLPatternImpl : public IsolateTracked { public: URLPatternImpl(url_pattern pattern); @@ -69,22 +70,8 @@ class URLPatternImpl { static void Exec(const v8::FunctionCallbackInfo& args); - void BindFinalizer(v8::Isolate* isolate, - const v8::Local& object) { - v8::HandleScope scopedHandle(isolate); - weakHandle_.Reset(isolate, object); - weakHandle_.SetWeak(this, Finalizer, v8::WeakCallbackType::kParameter); - } - - static void Finalizer(const v8::WeakCallbackInfo& data) { - auto* pThis = data.GetParameter(); - pThis->weakHandle_.Reset(); - delete pThis; - } - private: url_pattern pattern_; - v8::Global weakHandle_; static std::optional ParseInput( v8::Isolate* isolate, const v8::Local& input); diff --git a/NativeScript/runtime/URLSearchParamsImpl.h b/NativeScript/runtime/URLSearchParamsImpl.h index 25aa9560..87492ba4 100644 --- a/NativeScript/runtime/URLSearchParamsImpl.h +++ b/NativeScript/runtime/URLSearchParamsImpl.h @@ -4,11 +4,12 @@ #pragma once #include "Common.h" +#include "IsolateTracked.h" #include "ada/ada.h" namespace tns { -class URLSearchParamsImpl { +class URLSearchParamsImpl : public IsolateTracked { public: URLSearchParamsImpl(ada::url_search_params params); @@ -50,22 +51,8 @@ class URLSearchParamsImpl { static void Values(const v8::FunctionCallbackInfo& args); - void BindFinalizer(v8::Isolate* isolate, - const v8::Local& object) { - v8::HandleScope scopedHandle(isolate); - weakHandle_.Reset(isolate, object); - weakHandle_.SetWeak(this, Finalizer, v8::WeakCallbackType::kParameter); - } - - static void Finalizer(const v8::WeakCallbackInfo& data) { - auto* pThis = data.GetParameter(); - pThis->weakHandle_.Reset(); - delete pThis; - } - private: ada::url_search_params params_; - v8::Global weakHandle_; }; } // namespace tns