From 12ff142b415bcdee03f3d69310c4d18f19cf446d Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Thu, 12 Nov 2026 16:32:57 -0300 Subject: [PATCH 1/2] feat(runtime): wrapper lifetime diagnostics and released-object arguments A JS object whose ObjC wrapper was released (a resurrected holder's graph can keep it reachable) used to reach native code as a DictionaryAdapter standing in for the object, so UIKit received a dictionary where it expected, for example, a UIColor. A method call on it asserted in Release. Released ObjC wrappers now carry a ReleasedReason in internal field 0 (collected, released through __releaseNativeCounterpart, or teardown). GetValue only reads Externals, so existing callers are unaffected. Where an object is expected, the marker routes the value through releasedObjectPolicy: an argument becomes nil (report) or throws a ReferenceError (throw), a collection element becomes NSNull, a JS-implemented method's return value becomes nil, and a method or property call on it always throws a ReferenceError, also through a Proxy. ns:runtime gains getWrapperLifetimeStats() (resurrections, collections, explicit releases, released-object accesses, state-lost candidates and confirmations, counted on cold paths in every build) and the wrapperLifetimeWarnings key. With warnings on (default in debug builds, or "wrapperLifetime": { "warnings": bool } in the app config), a collected wrapper that carried JS properties while its object was still retained elsewhere is checked again a runloop turn later through a weak reference, and a still-alive object is reported once per class. --- NativeScript/runtime/ArgConverter.mm | 21 ++ NativeScript/runtime/Helpers.h | 34 +++ NativeScript/runtime/Helpers.mm | 121 +++++++++- NativeScript/runtime/Interop.mm | 17 ++ NativeScript/runtime/MetadataBuilder.mm | 6 + NativeScript/runtime/NsBuiltinModules.cpp | 51 +++- NativeScript/runtime/ObjectManager.h | 21 ++ NativeScript/runtime/ObjectManager.mm | 181 +++++++++++++- NativeScript/runtime/js/ns-runtime.js | 4 +- TestRunner/app/tests/NsRuntimeTests.js | 1 + TestRunner/app/tests/WrapperLifetimeTests.js | 241 +++++++++++++++++++ TestRunner/app/tests/index.js | 1 + docs/knowledge/v8-resurrecting-finalizers.md | 20 +- docs/ns-builtin-modules.md | 23 ++ types/ns-runtime.d.ts | 35 +++ 15 files changed, 757 insertions(+), 20 deletions(-) create mode 100644 TestRunner/app/tests/WrapperLifetimeTests.js diff --git a/NativeScript/runtime/ArgConverter.mm b/NativeScript/runtime/ArgConverter.mm index d5464db8..ef568b42 100644 --- a/NativeScript/runtime/ArgConverter.mm +++ b/NativeScript/runtime/ArgConverter.mm @@ -35,6 +35,19 @@ BaseDataWrapper* wrapper = tns::GetValue(isolate, receiver); if (wrapper == nullptr) { + tns::ReleasedReason releasedReason = tns::GetReleasedReason(receiver); + if (releasedReason != tns::ReleasedReason::kNone) { + const char* jsNameStr = meta ? meta->jsName() : ""; + std::string operation = std::string("method '") + jsNameStr + "' receiver"; + if (tns::ObjectManager::LifetimeStats* stats = + tns::ObjectManager::GetLifetimeStats(isolate)) { + stats->releasedAccesses++; + } + std::string message = + std::string(tns::ReleasedReasonMessage(releasedReason)) + " (" + operation + ")"; + throw NativeScriptException( + isolate, Exception::ReferenceError(tns::ToV8String(isolate, message)), message); + } // During fast view churn like HMR in development, JS objects can outlive their // native wrappers briefly. In Debug, throw a catchable JS error instead of crashing. // In Release, assert so crash reporting can capture unexpected cases. @@ -542,6 +555,14 @@ *static_cast(retValue) = data; return; } else { + // Native code receives this value outside any JS frame, so a released + // wrapper is reported (never thrown) and returned as nil. + tns::ReleasedReason releasedReason = tns::GetReleasedReason(value); + if (releasedReason != tns::ReleasedReason::kNone) { + tns::ReportReleasedObject(isolate, value, releasedReason, "method return value", false); + memset(retValue, 0, sizeof(id)); + return; + } id adapter = [[DictionaryAdapter alloc] initWithJSObject:tns::ProxyReadSource(original, value).As() isolate:isolate]; diff --git a/NativeScript/runtime/Helpers.h b/NativeScript/runtime/Helpers.h index d4fd42b8..4fbb59ba 100644 --- a/NativeScript/runtime/Helpers.h +++ b/NativeScript/runtime/Helpers.h @@ -316,6 +316,40 @@ void SetReleasedObjectPolicy(ReleasedObjectPolicy policy); // field unchecked must go through this. BaseDataWrapper* GetValueOrReport(v8::Isolate* isolate, const v8::Local& val, const char* operation); + +// Why an ObjC object's wrapper no longer has its native object. Stored as a +// small integer in internal field 0, where an External would otherwise sit: +// GetValue only reads Externals, so every existing caller sees "no wrapper", +// and writing it allocates nothing inside the finalizer drain. +enum class ReleasedReason : int32_t { + // Zero is never a reason: other templates' embedder fields can hold Smi 0. + kNone = 0, + // The wrapper's finalizer released it while JS could still reach the + // wrapper (only possible through a graph a resurrected wrapper keeps alive). + kCollected = 1, + // __releaseNativeCounterpart. + kExplicit = 2, + // Runtime teardown. + kTeardown = 3, +}; +void MarkReleased(v8::Isolate* isolate, const v8::Local& obj, ReleasedReason reason); +ReleasedReason GetReleasedReason(const v8::Local& val); +const char* ReleasedReasonMessage(ReleasedReason reason); + +// Applies ReleasedObjectPolicy to a released ObjC wrapper reaching native code +// in place of its object. Throws a ReferenceError under kThrow when `canThrow`; +// otherwise schedules the `releasednativeaccess` event (deduplicated per +// object). The caller then passes nil (or NSNull where nil is not allowed). +// Returns false when it threw. +bool ReportReleasedObject(v8::Isolate* isolate, const v8::Local& val, + ReleasedReason reason, const char* operation, bool canThrow); + +// Whether the runtime warns when JS state on a wrapper is lost while its native +// object lives on. Process-wide; defaults to the app's debug flag, overridden by +// the "wrapperLifetime": { "warnings": bool } app config or ns:runtime's +// `wrapperLifetimeWarnings` key. +bool WrapperLifetimeWarningsEnabled(); +void SetWrapperLifetimeWarnings(bool enabled); void DeleteValue(v8::Isolate* isolate, const v8::Local& val); bool DeleteWrapperIfUnused(v8::Isolate* isolate, const v8::Local& obj, BaseDataWrapper* value); diff --git a/NativeScript/runtime/Helpers.mm b/NativeScript/runtime/Helpers.mm index 41d5361b..f3f680c2 100644 --- a/NativeScript/runtime/Helpers.mm +++ b/NativeScript/runtime/Helpers.mm @@ -12,11 +12,13 @@ #include #include #include +#include #include #include #include "Caches.h" #include "ErrorEvents.h" #include "NativeScriptException.h" +#include "ObjectManager.h" #include "Runtime.h" #include "RuntimeConfig.h" @@ -519,23 +521,49 @@ void WriteDebugLine(tns::LogCategory category, const char* message) { va_end(ap); } -tns::BaseDataWrapper* tns::GetValueOrReport(Isolate* isolate, const Local& val, - const char* operation) { - BaseDataWrapper* wrapper = tns::GetValue(isolate, val); - if (wrapper != nullptr) { - return wrapper; +const char* tns::ReleasedReasonMessage(ReleasedReason reason) { + switch (reason) { + case tns::ReleasedReason::kCollected: + return "The native object was released when its wrapper was garbage collected"; + case tns::ReleasedReason::kExplicit: + return "The native object was released by __releaseNativeCounterpart"; + case tns::ReleasedReason::kTeardown: + return "The native object was released at runtime teardown"; + case tns::ReleasedReason::kNone: + break; } + return "The native object has been released or is not a valid native object"; +} - std::string message = - std::string("The native object has been released or is not a valid native object (") + - operation + ")"; +namespace { + +// Shared by GetValueOrReport and ReportReleasedObject: one message format, one +// event, one deduplication marker. +bool ApplyReleasedObjectPolicy(Isolate* isolate, const Local& val, const char* text, + const char* operation, bool canThrow) { + if (tns::ObjectManager::LifetimeStats* stats = tns::ObjectManager::GetLifetimeStats(isolate)) { + stats->releasedAccesses++; + } + std::string message = std::string(text) + " (" + operation + ")"; - if (GetReleasedObjectPolicy() == ReleasedObjectPolicy::kThrow) { + if (canThrow && tns::GetReleasedObjectPolicy() == tns::ReleasedObjectPolicy::kThrow) { isolate->ThrowException(v8::Exception::ReferenceError(tns::ToV8String(isolate, message))); - return nullptr; + return false; } Local context = isolate->GetCurrentContext(); + std::optional contextScope; + if (context.IsEmpty() && val->IsObject()) { + // Collection adapters convert elements from native callbacks that may not + // have entered a context. + context = tns::GetCreationContextOrCurrent(isolate, val.As()); + if (!context.IsEmpty()) { + contextScope.emplace(context); + } + } + if (context.IsEmpty()) { + return true; + } if (val->IsObject()) { // One report per husk object: a released buffer touched in a tight loop // must not flood the event queue. @@ -543,7 +571,7 @@ void WriteDebugLine(tns::LogCategory category, const char* message) { Private::ForApi(isolate, tns::ToV8String(isolate, "ns:releasedAccessReported")); Local obj = val.As(); if (obj->HasPrivate(context, reported).FromMaybe(false)) { - return nullptr; + return true; } obj->SetPrivate(context, reported, v8::True(isolate)).FromMaybe(false); } @@ -553,9 +581,80 @@ void WriteDebugLine(tns::LogCategory category, const char* message) { auto* report = new ReleasedAccessReport{isolate, v8::Global(isolate, error), operation}; isolate->EnqueueMicrotask(DispatchReleasedAccessMicrotask, report); + return true; +} + +std::atomic wrapperLifetimeWarnings_{-1}; + +} // namespace + +tns::BaseDataWrapper* tns::GetValueOrReport(Isolate* isolate, const Local& val, + const char* operation) { + BaseDataWrapper* wrapper = tns::GetValue(isolate, val); + if (wrapper != nullptr) { + return wrapper; + } + ApplyReleasedObjectPolicy(isolate, val, ReleasedReasonMessage(GetReleasedReason(val)), operation, + true); return nullptr; } +void tns::MarkReleased(Isolate* isolate, const Local& obj, ReleasedReason reason) { + Local target = tns::UnwrapProxy(obj); + if (!target.IsEmpty() && target->IsObject() && target.As()->InternalFieldCount() > 0) { + target.As()->SetInternalField(0, + v8::Integer::New(isolate, static_cast(reason))); + return; + } + // Wrappers kept in the `metadata` private have no field for the marker; they + // are only cleared, and read as plain objects afterwards. + tns::DeleteValue(isolate, obj); +} + +tns::ReleasedReason tns::GetReleasedReason(const Local& val) { + Local target = tns::UnwrapProxy(val); + if (target.IsEmpty() || !target->IsObject()) { + return ReleasedReason::kNone; + } + Local obj = target.As(); + if (obj->InternalFieldCount() < 1) { + return ReleasedReason::kNone; + } + Local field = obj->GetInternalField(0).As(); + if (field.IsEmpty() || !field->IsInt32()) { + return ReleasedReason::kNone; + } + return static_cast(field.As()->Value()); +} + +bool tns::ReportReleasedObject(Isolate* isolate, const Local& val, ReleasedReason reason, + const char* operation, bool canThrow) { + return ApplyReleasedObjectPolicy(isolate, val, ReleasedReasonMessage(reason), operation, canThrow); +} + +bool tns::WrapperLifetimeWarningsEnabled() { + int value = wrapperLifetimeWarnings_.load(std::memory_order_relaxed); + if (value >= 0) { + return value != 0; + } + bool enabled = RuntimeConfig.IsDebug; + id config = Runtime::GetAppConfigValue("wrapperLifetime"); + if ([config isKindOfClass:[NSDictionary class]]) { + id warnings = ((NSDictionary*)config)[@"warnings"]; + if ([warnings isKindOfClass:[NSNumber class]]) { + enabled = [warnings boolValue]; + } + } + int expected = -1; + wrapperLifetimeWarnings_.compare_exchange_strong(expected, enabled ? 1 : 0, + std::memory_order_relaxed); + return wrapperLifetimeWarnings_.load(std::memory_order_relaxed) != 0; +} + +void tns::SetWrapperLifetimeWarnings(bool enabled) { + wrapperLifetimeWarnings_.store(enabled ? 1 : 0, std::memory_order_relaxed); +} + void tns::DeleteValue(Isolate* isolate, const Local& val) { Local target = tns::UnwrapProxy(val); if (target.IsEmpty() || target->IsNullOrUndefined() || !target->IsObject()) { diff --git a/NativeScript/runtime/Interop.mm b/NativeScript/runtime/Interop.mm index 19671455..d140dba7 100644 --- a/NativeScript/runtime/Interop.mm +++ b/NativeScript/runtime/Interop.mm @@ -776,6 +776,16 @@ static bool SnapshotArrayLike(Isolate* isolate, Local object, Localtype == BinaryTypeEncodingType::InterfaceDeclarationReference) { std::string name = typeEncoding->details.interfaceDeclarationReference.name.valuePtr(); @@ -866,6 +876,13 @@ static bool SnapshotArrayLike(Isolate* isolate, Local object, Local() isolate:isolate]; // CFAutorelease(adapter); diff --git a/NativeScript/runtime/MetadataBuilder.mm b/NativeScript/runtime/MetadataBuilder.mm index d45d980c..2cc3e9b5 100644 --- a/NativeScript/runtime/MetadataBuilder.mm +++ b/NativeScript/runtime/MetadataBuilder.mm @@ -54,6 +54,12 @@ bool ResolveProxyReceiver(Isolate* isolate, Local& receiver, const char* (wrapper->Type() == WrapperType::ObjCObject || wrapper->Type() == WrapperType::ObjCAllocObject); if (!isClass && !isInstance) { + if (tns::GetReleasedReason(target) != tns::ReleasedReason::kNone) { + // A released wrapper: the call reports it the same way as on the + // target directly. + receiver = target.As(); + return true; + } reason = "on a Proxy whose target is not a native object"; } } diff --git a/NativeScript/runtime/NsBuiltinModules.cpp b/NativeScript/runtime/NsBuiltinModules.cpp index 902626c9..200afb3c 100644 --- a/NativeScript/runtime/NsBuiltinModules.cpp +++ b/NativeScript/runtime/NsBuiltinModules.cpp @@ -8,6 +8,7 @@ #include "Helpers.h" #include "Interop.h" #include "Messaging.h" +#include "ObjectManager.h" #include "ModuleInternalCallbacks.h" #include "Runtime.h" #include "StructuredSerialization.h" @@ -101,6 +102,7 @@ std::string Canonicalize(const std::string& specifier) { // deliberately not registered — it is boot-time nativescript.config only. constexpr const char* kReleasedObjectPolicyKey = "releasedObjectPolicy"; constexpr const char* kDebugKey = "debug"; +constexpr const char* kWrapperLifetimeWarningsKey = "wrapperLifetimeWarnings"; void ThrowTypeError(Isolate* isolate, const std::string& message) { isolate->ThrowException( @@ -139,6 +141,17 @@ void SetConfigCallback(const FunctionCallbackInfo& info) { } return; } + if (key == kWrapperLifetimeWarningsKey) { + if (!EnsureMainIsolateWrite(isolate, key)) { + return; + } + if (!info[1]->IsBoolean()) { + ThrowTypeError(isolate, "'" + key + "' must be a boolean"); + return; + } + tns::SetWrapperLifetimeWarnings(info[1]->IsTrue()); + return; + } if (key == kDebugKey) { if (!EnsureMainIsolateWrite(isolate, key)) { return; @@ -187,9 +200,40 @@ void GetConfigCallback(const FunctionCallbackInfo& info) { tns::ToV8String(isolate, tns::EnabledLogCategoryNames())); return; } + if (key == kWrapperLifetimeWarningsKey) { + info.GetReturnValue().Set(tns::WrapperLifetimeWarningsEnabled()); + return; + } ThrowTypeError(isolate, "Unknown runtime config key: '" + key + "'"); } +// Counters for the calling isolate; see ObjectManager::LifetimeStats. +void GetWrapperLifetimeStatsCallback(const FunctionCallbackInfo& info) { + Isolate* isolate = info.GetIsolate(); + Local context = isolate->GetCurrentContext(); + Local result = Object::New(isolate); + tns::ObjectManager::LifetimeStats* stats = tns::ObjectManager::GetLifetimeStats(isolate); + if (stats == nullptr) { + info.GetReturnValue().Set(result); + return; + } + const std::pair fields[] = { + {"resurrections", stats->resurrections}, + {"collected", stats->collected}, + {"releasedExplicitly", stats->releasedExplicitly}, + {"releasedAccesses", stats->releasedAccesses}, + {"stateLostCandidates", stats->stateLostCandidates}, + {"stateLostConfirmed", stats->stateLostConfirmed}, + }; + for (const auto& [name, value] : fields) { + result + ->Set(context, tns::ToV8String(isolate, name), + v8::Number::New(isolate, static_cast(value))) + .FromMaybe(false); + } + info.GetReturnValue().Set(result); +} + const Registration* Find(const std::string& specifier) { for (const Registration& registration : kRegistry) { if (specifier == registration.specifier) { @@ -218,12 +262,17 @@ MaybeLocal NsModuleBinding(Local context) { MaybeLocal NsRuntimeBinding(Local context) { Isolate* isolate = v8::Isolate::GetCurrent(); Local binding = Object::New(isolate); - Local setConfig, getConfig; + Local setConfig, getConfig, getWrapperLifetimeStats; if (!v8::Function::New(context, SetConfigCallback).ToLocal(&setConfig) || !v8::Function::New(context, GetConfigCallback).ToLocal(&getConfig) || + !v8::Function::New(context, GetWrapperLifetimeStatsCallback) + .ToLocal(&getWrapperLifetimeStats) || !binding->Set(context, tns::ToV8String(isolate, "setConfig"), setConfig) .FromMaybe(false) || !binding->Set(context, tns::ToV8String(isolate, "getConfig"), getConfig) + .FromMaybe(false) || + !binding->Set(context, tns::ToV8String(isolate, "getWrapperLifetimeStats"), + getWrapperLifetimeStats) .FromMaybe(false)) { return MaybeLocal(); } diff --git a/NativeScript/runtime/ObjectManager.h b/NativeScript/runtime/ObjectManager.h index 67691ff3..046eaff1 100644 --- a/NativeScript/runtime/ObjectManager.h +++ b/NativeScript/runtime/ObjectManager.h @@ -42,6 +42,27 @@ struct ObjectWeakCallbackState { class ObjectManager { public: + // Per-isolate wrapper lifetime counters. Kept in every build: plain integers + // bumped only on cold paths (finalizers, released-object reports), exposed + // through ns:runtime's getWrapperLifetimeStats(). + struct LifetimeStats { + // An ObjC wrapper's finalizer refused disposal because native code still + // retains its object (IsGcProtected), keeping the wrapper alive. + uint64_t resurrections = 0; + // An ObjC wrapper's finalizer released its native object. + uint64_t collected = 0; + // __releaseNativeCounterpart released a native object. + uint64_t releasedExplicitly = 0; + // JS handed a released wrapper to native code or used it as a receiver. + uint64_t releasedAccesses = 0; + // A finalized ObjC wrapper carried JS properties while something other + // than the wrapper still retained its object, and whether that object was + // still alive a runloop turn later (see WrapperLifetimeWarningsEnabled). + uint64_t stateLostCandidates = 0; + uint64_t stateLostConfirmed = 0; + }; + static LifetimeStats* GetLifetimeStats(v8::Isolate* isolate); + static void Init(v8::Isolate* isolate, v8::Local globalTemplate); static std::shared_ptr> Register( diff --git a/NativeScript/runtime/ObjectManager.mm b/NativeScript/runtime/ObjectManager.mm index 904ab233..41d71775 100644 --- a/NativeScript/runtime/ObjectManager.mm +++ b/NativeScript/runtime/ObjectManager.mm @@ -7,6 +7,17 @@ #include "DataWrapper.h" #include "FFICall.h" #include "Helpers.h" +#include "Runtime.h" +#include +#include +#include +#include +#include + +// Exported by libobjc; not declared in a public header for MRC code. +extern "C" id objc_initWeak(id* location, id value); +extern "C" id objc_loadWeakRetained(id* location); +extern "C" void objc_destroyWeak(id* location); using namespace v8; using namespace std; @@ -15,6 +26,133 @@ static Class NSTimerClass = objc_getClass("NSTimer"); +namespace { + +// A finalized wrapper that carried JS properties while something other than +// the wrapper still retained its object. Confirmed a runloop turn later, so a +// co-queued owner or a pending autorelease does not produce a warning. +struct StateLostCandidate { + id* weakSlot; + std::string description; + std::string className; + std::string properties; +}; + +struct WrapperLifetimeState { + ObjectManager::LifetimeStats stats; + std::vector candidates; + bool confirmPosted = false; + std::unordered_set warnedClasses; + + ~WrapperLifetimeState() { + for (StateLostCandidate& candidate : candidates) { + objc_destroyWeak(candidate.weakSlot); + delete candidate.weakSlot; + } + } +}; + +bool IsTaggedPointer(id obj) { +#if defined(__arm64__) + return reinterpret_cast(obj) < 0; +#else + return (reinterpret_cast(obj) & 1) != 0; +#endif +} + +void ConfirmStateLost(Isolate* isolate) { + auto* state = Caches::StateFor(isolate); + if (state == nullptr) { + return; + } + state->confirmPosted = false; + std::vector candidates; + candidates.swap(state->candidates); + for (StateLostCandidate& candidate : candidates) { + id obj = objc_loadWeakRetained(candidate.weakSlot); + if (obj != nil) { + state->stats.stateLostConfirmed++; + if (state->warnedClasses.insert(candidate.className).second) { + Log("NativeScript: JS properties (%s) set on %s were lost when its JavaScript wrapper was " + "garbage collected, but the native object is still alive. If native code hands it " + "back, it gets a new wrapper without them. Keep a JS reference to the object while " + "its JS state is needed. Reported once per class; disable with \"wrapperLifetime\": " + "{ \"warnings\": false }.", + candidate.properties.c_str(), candidate.description.c_str()); + } + [obj release]; + } + objc_destroyWeak(candidate.weakSlot); + delete candidate.weakSlot; + } +} + +// Called from the finalizer drain just before the wrapper releases `target`. +void NoteStateLostCandidate(Isolate* isolate, Local obj, id target) { + if (IsTaggedPointer(target)) { + return; + } + NSUInteger count = [target retainCount]; + if (count <= 1 || count >= (NSUIntegerMax >> 1)) { + return; + } + SEL allowsWeak = sel_registerName("allowsWeakReference"); + if ([target respondsToSelector:allowsWeak] && + !reinterpret_cast(objc_msgSend)(target, allowsWeak)) { + return; + } + + HandleScope handleScope(isolate); + TryCatch tryCatch(isolate); + Local context = tns::GetCreationContextOrCurrent(isolate, obj); + Local names; + if (context.IsEmpty() || + !obj->GetOwnPropertyNames(context, PropertyFilter::SKIP_SYMBOLS).ToLocal(&names) || + names->Length() == 0) { + return; + } + + auto* state = Caches::StateFor(isolate); + if (state == nullptr) { + return; + } + state->stats.stateLostCandidates++; + + std::string properties; + uint32_t shown = std::min(names->Length(), 5); + for (uint32_t i = 0; i < shown; i++) { + Local name; + if (!names->Get(context, i).ToLocal(&name)) { + continue; + } + if (!properties.empty()) { + properties += ", "; + } + properties += tns::ToString(isolate, name); + } + if (names->Length() > shown) { + properties += ", ..."; + } + + const char* className = object_getClassName(target); + char description[256]; + snprintf(description, sizeof(description), "<%s: %p>", className, (void*)target); + + id* slot = new id(nil); + objc_initWeak(slot, target); + state->candidates.push_back({slot, description, className, properties}); + + if (!state->confirmPosted) { + Runtime* runtime = Runtime::GetRuntime(isolate); + std::shared_ptr loop = runtime != nullptr ? runtime->GetEventLoop() : nullptr; + if (loop != nullptr && loop->PostInternal([isolate]() { ConfirmStateLost(isolate); })) { + state->confirmPosted = true; + } + } +} + +} // namespace + void ObjectManager::Init(Isolate* isolate, Local globalTemplate) { globalTemplate->Set(tns::ToV8String(isolate, "__releaseNativeCounterpart"), FunctionTemplate::New(isolate, ReleaseNativeCounterpartCallback)); @@ -192,16 +330,24 @@ void DisposeHandle(v8::Isolate* isolate, BaseDataWrapper* wrapper = tns::GetValue(isolate, value); // NSLog(@"dispose %p", wrapper); if (wrapper == nullptr) { - tns::SetValue(isolate, obj, nullptr); + if (tns::GetReleasedReason(obj) == tns::ReleasedReason::kNone) { + tns::SetValue(isolate, obj, nullptr); + } return true; } + const WrapperType type = wrapper->Type(); if (wrapper->IsGcProtected() && !isFinalDisposal) { + if (type == WrapperType::ObjCObject) { + if (LifetimeStats* stats = GetLifetimeStats(isolate)) { + stats->resurrections++; + } + } return false; } std::shared_ptr cache = Caches::Get(isolate); - switch (wrapper->Type()) { + switch (type) { case WrapperType::Struct: // Lives in the struct object's ArrayBuffer storage, which the GC frees. return true; @@ -219,6 +365,19 @@ void DisposeHandle(v8::Isolate* isolate, cache->Instances.erase(it); } } + if (!isFinalDisposal) { + if (LifetimeStats* stats = GetLifetimeStats(isolate)) { + stats->collected++; + } + if (tns::WrapperLifetimeWarningsEnabled()) { + NoteStateLostCandidate(isolate, obj, target); + // It allocates, so a nested collection may have run finalizers + // whose JS released this object explicitly. + if (tns::GetValue(isolate, obj) != wrapper) { + return true; + } + } + } [target release]; } break; @@ -313,7 +472,13 @@ void DisposeHandle(v8::Isolate* isolate, // so only a wrapper still sitting in it is ours to free. if (tns::GetValue(isolate, obj) == wrapper) { delete wrapper; - tns::DeleteValue(isolate, obj); + if (type == WrapperType::ObjCObject) { + tns::MarkReleased(isolate, obj, + isFinalDisposal ? tns::ReleasedReason::kTeardown + : tns::ReleasedReason::kCollected); + } else { + tns::DeleteValue(isolate, obj); + } } return true; } @@ -396,13 +561,21 @@ void DisposeHandle(v8::Isolate* isolate, // again. if (!adapterClaim && tns::GetValue(isolate, value) == wrapper) { delete wrapper; - tns::SetValue(isolate, value.As(), nullptr); + tns::MarkReleased(isolate, value.As(), tns::ReleasedReason::kExplicit); + } + if (LifetimeStats* stats = GetLifetimeStats(isolate)) { + stats->releasedExplicitly++; } } } bool ObjectManager::IsInstanceOf(id obj, Class clazz) { return [obj isKindOfClass:clazz]; } +ObjectManager::LifetimeStats* ObjectManager::GetLifetimeStats(Isolate* isolate) { + auto* state = Caches::StateFor(isolate); + return state != nullptr ? &state->stats : nullptr; +} + long ObjectManager::GetRetainCount(id obj) { if (!obj) { return 0; diff --git a/NativeScript/runtime/js/ns-runtime.js b/NativeScript/runtime/js/ns-runtime.js index 03abfd7a..f34642f5 100644 --- a/NativeScript/runtime/js/ns-runtime.js +++ b/NativeScript/runtime/js/ns-runtime.js @@ -1,7 +1,7 @@ "use strict"; // The `ns:runtime` builtin module: runtime-level configuration, native -// exception escaping and external-memory accounting. See +// exception escaping, external-memory accounting and wrapper lifetime counters. See // docs/ns-builtin-modules.md for the contract and the key registry — keys, // their value domains, and their scope (process-wide vs per-isolate) are // defined and validated on the native side, so this file stays a thin, frozen @@ -13,6 +13,7 @@ const { escapeException, setExternalSize, getExternalSize, + getWrapperLifetimeStats, } = binding; const { ObjectFreeze } = primordials; @@ -21,4 +22,5 @@ exports.getConfig = getConfig; exports.escapeException = escapeException; exports.setExternalSize = setExternalSize; exports.getExternalSize = getExternalSize; +exports.getWrapperLifetimeStats = getWrapperLifetimeStats; ObjectFreeze(exports); diff --git a/TestRunner/app/tests/NsRuntimeTests.js b/TestRunner/app/tests/NsRuntimeTests.js index 1b1b2868..3b2623d0 100644 --- a/TestRunner/app/tests/NsRuntimeTests.js +++ b/TestRunner/app/tests/NsRuntimeTests.js @@ -14,6 +14,7 @@ describe("ns:runtime", function () { "escapeException", "getConfig", "getExternalSize", + "getWrapperLifetimeStats", "setConfig", "setExternalSize", ]); diff --git a/TestRunner/app/tests/WrapperLifetimeTests.js b/TestRunner/app/tests/WrapperLifetimeTests.js new file mode 100644 index 00000000..0ebeb62f --- /dev/null +++ b/TestRunner/app/tests/WrapperLifetimeTests.js @@ -0,0 +1,241 @@ +describe("Wrapper lifetime", function () { + var runtime = require("ns:runtime"); + + function releasedObject() { + var obj = NSObject.alloc().init(); + __releaseNativeCounterpart(obj); + return obj; + } + + function stats() { + return runtime.getWrapperLifetimeStats(); + } + + // The state-lost confirmation runs on the event loop's internal lane, not + // necessarily before the next timer. + function afterTurns(turns, callback) { + setTimeout(function () { + if (turns > 1) { + afterTurns(turns - 1, callback); + } else { + callback(); + } + }, 10); + } + + afterEach(function () { + runtime.setConfig("releasedObjectPolicy", "report"); + }); + + describe("released objects reaching native code", function () { + it("passes nil, not a dictionary, for a released argument under the report policy", function (done) { + var released = releasedObject(); + var events = []; + var onReleased = function (e) { + events.push(e); + }; + global.addEventListener("releasednativeaccess", onReleased); + + var dict = NSMutableDictionary.new(); + dict.setValueForKey(released, "key"); + expect(dict.count).toBe(0); + + setTimeout(function () { + global.removeEventListener("releasednativeaccess", onReleased); + expect(events.length).toBe(1); + expect(events[0].error.message).toMatch(/__releaseNativeCounterpart/); + expect(events[0].error.message).toMatch(/native argument/); + done(); + }); + }); + + it("throws a ReferenceError for a released argument under the throw policy", function () { + var released = releasedObject(); + runtime.setConfig("releasedObjectPolicy", "throw"); + var dict = NSMutableDictionary.new(); + expect(function () { + dict.setValueForKey(released, "key"); + }).toThrowError(ReferenceError, /released by __releaseNativeCounterpart/); + expect(dict.count).toBe(0); + }); + + it("uses NSNull for a released collection element", function () { + var released = releasedObject(); + var array = NSArray.arrayWithArray([released]); + expect(array.count).toBe(1); + var element = array.objectAtIndex(0); + expect(element === null || element instanceof NSNull).toBe(true); + }); + + it("throws a ReferenceError when a released object is a method receiver", function () { + var released = releasedObject(); + expect(function () { + released.isEqual(null); + }).toThrowError(ReferenceError, /released by __releaseNativeCounterpart/); + }); + + it("throws a ReferenceError when reading a property of a released object", function () { + var released = releasedObject(); + expect(function () { + return released.description; + }).toThrowError(ReferenceError, /released by __releaseNativeCounterpart/); + }); + + it("marks the target when released through a Proxy", function () { + var target = NSObject.alloc().init(); + var proxy = new Proxy(target, {}); + __releaseNativeCounterpart(proxy); + expect(function () { + target.isEqual(null); + }).toThrowError(ReferenceError, /released by __releaseNativeCounterpart/); + expect(function () { + proxy.isEqual(null); + }).toThrowError(ReferenceError, /released by __releaseNativeCounterpart/); + }); + + it("counts explicit releases and released accesses", function () { + var before = stats(); + var released = releasedObject(); + expect(stats().releasedExplicitly).toBe(before.releasedExplicitly + 1); + try { + released.isEqual(null); + } catch (e) { + } + expect(stats().releasedAccesses).toBe(before.releasedAccesses + 1); + }); + }); + + // A natively retained JS-extended object is resurrected by its finalizer, + // while a native object referenced only from its JS state is released in + // the same collection: the shape behind native crashes where the released + // object reached UIKit as a dictionary. Zero husks is a legal (if unlucky) + // outcome, as in GCFinalizerTests. + it("names garbage collection as the reason for an object released under a resurrected holder", function () { + var Holder = (function (_super) { + __extends(Holder, _super); + function Holder() { + return (_super !== null && _super.apply(this, arguments)) || this; + } + return Holder; + })(NSObject); + + var N = 16; + var owner = NSMutableArray.alloc().init(); + var before = stats(); + (function create() { + for (var i = 0; i < N; i++) { + var holder = Holder.alloc().init(); + holder.state = { item: NSObject.alloc().init() }; + owner.addObject(holder); + } + })(); + (function scrub(n) { + return n > 0 ? scrub(n - 1) + n : 0; + })(300); + __collect(); + __collect(); + + expect(stats().resurrections).toBeGreaterThan(before.resurrections); + + var husks = 0; + for (var i = 0; i < N; i++) { + var holder = owner.objectAtIndex(i); + if (holder.state === undefined) { + continue; + } + try { + holder.state.item.isEqual(null); + } catch (e) { + expect(e instanceof ReferenceError).toBe(true); + expect(e.message).toMatch(/garbage collected/); + husks++; + } + } + TNSLog("wrapper lifetime spec: " + husks + "/" + N + " husks formed"); + }); + + it("reports JS state lost on a wrapper whose native object lives on", function (done) { + var previous = runtime.getConfig("wrapperLifetimeWarnings"); + runtime.setConfig("wrapperLifetimeWarnings", true); + var N = 16; + var owner = NSMutableArray.alloc().init(); + var before = stats(); + (function create() { + for (var i = 0; i < N; i++) { + var obj = NSObject.alloc().init(); + obj.jsState = i; + owner.addObject(obj); + } + })(); + (function scrub(n) { + return n > 0 ? scrub(n - 1) + n : 0; + })(300); + __collect(); + __collect(); + + afterTurns(5, function () { + var after = stats(); + runtime.setConfig("wrapperLifetimeWarnings", previous); + expect(after.stateLostCandidates).toBeGreaterThan(before.stateLostCandidates); + expect(after.stateLostConfirmed).toBeGreaterThan(before.stateLostConfirmed); + expect(owner.objectAtIndex(0).jsState).toBeUndefined(); + done(); + }); + }); + + it("does not report state lost when nothing native keeps the object", function (done) { + var previous = runtime.getConfig("wrapperLifetimeWarnings"); + runtime.setConfig("wrapperLifetimeWarnings", true); + // Settle earlier specs' garbage first, so only this spec's objects + // can move the counter. + __collect(); + __collect(); + afterTurns(5, function () { + var before = stats(); + (function create() { + for (var i = 0; i < 16; i++) { + var obj = NSObject.alloc().init(); + obj.jsState = i; + } + })(); + (function scrub(n) { + return n > 0 ? scrub(n - 1) + n : 0; + })(300); + __collect(); + __collect(); + + afterTurns(5, function () { + var after = stats(); + runtime.setConfig("wrapperLifetimeWarnings", previous); + expect(after.stateLostConfirmed).toBe(before.stateLostConfirmed); + done(); + }); + }); + }); + + it("exposes numeric counters", function () { + var current = stats(); + expect(Object.keys(current).sort()).toEqual([ + "collected", + "releasedAccesses", + "releasedExplicitly", + "resurrections", + "stateLostCandidates", + "stateLostConfirmed", + ]); + Object.keys(current).forEach(function (key) { + expect(typeof current[key]).toBe("number"); + }); + }); + + it("validates the wrapperLifetimeWarnings config key", function () { + var previous = runtime.getConfig("wrapperLifetimeWarnings"); + expect(typeof previous).toBe("boolean"); + expect(function () { + runtime.setConfig("wrapperLifetimeWarnings", "yes"); + }).toThrowError(TypeError, /'wrapperLifetimeWarnings' must be a boolean/); + runtime.setConfig("wrapperLifetimeWarnings", !previous); + expect(runtime.getConfig("wrapperLifetimeWarnings")).toBe(!previous); + runtime.setConfig("wrapperLifetimeWarnings", previous); + }); +}); diff --git a/TestRunner/app/tests/index.js b/TestRunner/app/tests/index.js index 9c9ce44f..376b1688 100644 --- a/TestRunner/app/tests/index.js +++ b/TestRunner/app/tests/index.js @@ -134,6 +134,7 @@ require("./NsWorkerThreadsTests"); require("./NsUrlTests"); require("./NsInteropTests"); require("./GCFinalizerTests"); +require("./WrapperLifetimeTests"); require("./BlockCacheRaceTests"); require("./WorkerConcurrentStartupTests"); require("./WorkerOptionsTests"); diff --git a/docs/knowledge/v8-resurrecting-finalizers.md b/docs/knowledge/v8-resurrecting-finalizers.md index ece736d6..ddde5c40 100644 --- a/docs/knowledge/v8-resurrecting-finalizers.md +++ b/docs/knowledge/v8-resurrecting-finalizers.md @@ -157,9 +157,12 @@ both JS-unreachable and one references the other, **both** are queued: - `n1`'s callback finds `IsGcProtected()` and re-arms. `n1` comes back holding a JS object whose native half is gone. `DisposeValue` does neuter the -husk (`delete wrapper` then `tns::DeleteValue`, which sets internal field 0 to `Undefined`), so -it is not a dangling `BaseDataWrapper*` — but consumers that read the field without checking, -`Pointer.cpp` among them, will read `Undefined` as an `External` and dereference it. +husk (`delete wrapper`, then internal field 0 is overwritten), so it is not a dangling +`BaseDataWrapper*` — but consumers that read the field without checking, `Pointer.cpp` among +them, would read it as an `External` and dereference it. ObjC object husks get a +`tns::ReleasedReason` Smi in the field (`tns::MarkReleased`: collected, released explicitly +through `__releaseNativeCounterpart`, or teardown); other wrapper types get `Undefined`. +`tns::GetValue` only reads `External`s, so both read as "no wrapper". **This is not fixable by deferring the release or by re-checking liveness after the drain.** Every queued node is rooted by `IterateFinalizerHandlesAsRoots()` and treated as a strong @@ -195,6 +198,17 @@ both struct field accessors, the two `InteropTypes.mm` argument readers, and the get/set, toString). Regression spec: *"throws instead of crashing when touching a released object through a resurrected parent"* in `GCFinalizerTests.js`. +An ObjC object husk reaching native code where an object is expected used to fall through to a +`DictionaryAdapter` standing in for it (a released `UIColor` reached UIKit as one and raised +`unrecognized selector`). The `ReleasedReason` marker now catches it on those paths, through +`tns::ReportReleasedObject()` with the same policy: an argument (`Interop::WriteValue`) becomes +`nil` or throws; a collection element (`Interop::ToObject`) becomes `NSNull`; a JS-implemented +method's return value (`ArgConverter::SetValue`) becomes `nil` (both report only, since native +code is on the stack); a method or property call on the husk (`ArgConverter::Invoke`, Proxy +receivers included) always throws. `ns:runtime`'s `getWrapperLifetimeStats()` counts +resurrections, collections, explicit releases and released-object accesses. Specs: +`WrapperLifetimeTests.js`. + Two facts worth knowing when reasoning about which objects can husk: - `Pointer`/`interop.alloc` objects never husk at runtime — `Caches::PointerInstances` diff --git a/docs/ns-builtin-modules.md b/docs/ns-builtin-modules.md index 060b5eff..1efc29b6 100644 --- a/docs/ns-builtin-modules.md +++ b/docs/ns-builtin-modules.md @@ -103,6 +103,7 @@ defined and validated natively; the module surface is a thin frozen wrapper. | `escapeException(error)` | Returns an `Error` branded to cross the next JS-to-native boundary as a native exception rather than being reported in JS (see [error-handling.md](error-handling.md#forwarding-a-throw-to-native-interopescapeexception)). An already-branded value is returned unchanged; no argument throws `TypeError`. | | `setExternalSize(target, bytes)` | Reports `bytes` of native memory held by a native object, pointer, reference, block or function reference to the garbage collector, replacing any earlier size (`0` clears it). Throws `TypeError` for any other value, `RangeError` for a byte count outside 0–2^34. | | `getExternalSize(target)` | The size currently charged to `target`, `0` when there is none. | +| `getWrapperLifetimeStats()` | Wrapper lifetime counters for the calling isolate (see [Wrapper lifetime counters](#wrapper-lifetime-counters)). | The last three are also members of the global `interop` (see [The global `interop`](#the-global-interop)); the module and the global hold @@ -114,6 +115,7 @@ Config keys: |---|---|---|---| | `releasedObjectPolicy` | `"report"` \| `"throw"` | process-wide (main-isolate writes only; read live by every isolate) | `"report"` | | `debug` | comma-separated category list, e.g. `"esm,fetch"` | process-wide (main-isolate writes only; read live by every isolate) | the `NS_DEBUG` environment variable, or `""` | +| `wrapperLifetimeWarnings` | `true` \| `false` | process-wide (main-isolate writes only; read live by every isolate) | the app config `"wrapperLifetime": { "warnings": … }`, or on in debug builds | ```js const { setConfig, getConfig } = require("ns:runtime"); @@ -141,6 +143,27 @@ The `TypeError` messages are part of the contract: | process-wide key written from a worker | `'' is process-wide and can only be set from the main isolate` | | invalid `releasedObjectPolicy` value | `'releasedObjectPolicy' must be 'report' or 'throw'` | | non-string `debug` value | `'debug' must be a comma-separated category string (), or '' to disable tracing` | +| non-boolean `wrapperLifetimeWarnings` value | `'wrapperLifetimeWarnings' must be a boolean` | + +### Wrapper lifetime counters + +`getWrapperLifetimeStats()` returns counters for the calling isolate, kept in +every build (they only cost an increment on finalizer and error paths): + +| counter | counts | +|---|---| +| `resurrections` | finalizers that kept a wrapper alive because native code still retained its object (JS-extended classes) | +| `collected` | finalizers that released their native object | +| `releasedExplicitly` | native objects released through `__releaseNativeCounterpart` | +| `releasedAccesses` | times JS passed a released wrapper to native code, used it as a method receiver, or touched a released pointer, reference or struct | +| `stateLostCandidates` | collected wrappers carrying JS properties whose object was still retained by something else (only while `wrapperLifetimeWarnings` is on) | +| `stateLostConfirmed` | candidates whose object was still alive a runloop turn later; each class is also reported once in the log | + +A wrapper whose native object was released keeps its JS identity. Passing it +to native code follows `releasedObjectPolicy`: under `"report"` the argument +or return value becomes `nil` (`NSNull` inside a collection) and a +`releasednativeaccess` event fires; under `"throw"` an argument throws a +`ReferenceError`. Calling a method on it always throws a `ReferenceError`. Remote-module security (`security.allowRemoteModules`, `security.remoteModuleAllowlist`) is **not** part of this surface. Those diff --git a/types/ns-runtime.d.ts b/types/ns-runtime.d.ts index 37a74c68..b60ea136 100644 --- a/types/ns-runtime.d.ts +++ b/types/ns-runtime.d.ts @@ -31,6 +31,35 @@ declare module "ns:runtime" { * Available in release builds too. Process-wide; main-isolate writes only. */ debug: string; + /** + * Whether the runtime logs a warning (once per class) when JS properties + * on a native object's wrapper are lost because the wrapper was garbage + * collected while native code still owned the object. + * + * Defaults to on in debug builds and off otherwise; the app config key + * `"wrapperLifetime": { "warnings": boolean }` overrides the default. + * Process-wide; main-isolate writes only. + */ + wrapperLifetimeWarnings: boolean; + } + + /** + * Wrapper lifetime counters for the calling isolate, maintained in every + * build. + */ + export interface WrapperLifetimeStats { + /** Finalizers that kept a wrapper alive because native code still retained its object. */ + resurrections: number; + /** Finalizers that released their native object. */ + collected: number; + /** Native objects released through `__releaseNativeCounterpart`. */ + releasedExplicitly: number; + /** Times JS passed a released wrapper to native code or called a method on it. */ + releasedAccesses: number; + /** Collected wrappers with JS properties whose object was still retained elsewhere. */ + stateLostCandidates: number; + /** Candidates whose object was still alive a runloop turn later (warnings enabled only). */ + stateLostConfirmed: number; } /** @@ -81,4 +110,10 @@ declare module "ns:runtime" { * runtime itself, `0` when there is none. */ export function getExternalSize(target: unknown): number; + + /** + * Wrapper lifetime counters for the calling isolate. The state-lost + * counters only advance while `wrapperLifetimeWarnings` is on. + */ + export function getWrapperLifetimeStats(): WrapperLifetimeStats; } From d5016e90cf9fed956f3aca1c688ea3fec6be4666 Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Thu, 12 Nov 2026 16:58:46 -0300 Subject: [PATCH 2/2] fix(runtime): test ObjC tagged pointers through void* for Release builds clang rejects bit tests on ObjC object pointers under -Werror (-Wdeprecated-objc-pointer-introspection), which broke the x86_64 simulator archive. --- NativeScript/runtime/ObjectManager.mm | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/NativeScript/runtime/ObjectManager.mm b/NativeScript/runtime/ObjectManager.mm index 41d71775..5d5c79ae 100644 --- a/NativeScript/runtime/ObjectManager.mm +++ b/NativeScript/runtime/ObjectManager.mm @@ -53,10 +53,12 @@ }; bool IsTaggedPointer(id obj) { + // Through void*: clang rejects bit tests on ObjC object pointers. + uintptr_t bits = reinterpret_cast(static_cast(obj)); #if defined(__arm64__) - return reinterpret_cast(obj) < 0; + return (bits >> 63) != 0; #else - return (reinterpret_cast(obj) & 1) != 0; + return (bits & 1) != 0; #endif }