Runtime lifetime: deferred leaks, cross-isolate sharing, and startup robustness

[edusperoni]

Tracking issue for lifetime problems found while fixing the intermittent worker SIGSEGV (#2006) and the ObjectManager teardown (#2008), and deliberately not fixed there. Each was verified by reading the code; none is speculative. Line numbers are as of fix/objectmanager-teardown.

Two facts underpin most of these, and are worth stating once:

• V8 does not run weak callbacks when an isolate is disposed. Anything that frees itself only from a GC finalizer leaks every instance that is still alive when a runtime goes away — and workers make that happen constantly.
• v8::Persistent and v8::Global differ at destruction. Persistent uses NonCopyablePersistentTraits (kResetInDestructor == false), so ~Persistent never calls into V8 and delete po after Isolate::Dispose() is safe. Global always resets, so destroying one after disposal writes into a freed handle table. That single difference decides which teardown window each fix belongs in.

The three windows, from WorkerWrapper.cpp:461-503:

Window	Isolate	JNI	Use for
Runtime::DestroyRuntime()	alive, locked	alive	anything touching v8::Global / Persistent::Reset / JS objects
~Runtime()	disposed	alive, attached	JNI ref release, delete of Persistent<T>*
after DetachCurrentThread	gone	detached	nothing

---

Startup robustness

• Main-runtime initialization and election are not serialized. initRuntime (Runtime.java:572-576) calls the synchronized (Runtime.currentRuntime) constructor and then runtime.init() — and init(), which reaches initNativeScript → PrepareV8Runtime, is outside that block. So the s_mainThreadInitialized check-then-act is not protected: two concurrent bootstraps could both run InitializeV8(), both elect a main runtime, and overwrite Runtime::platform / s_mainEventLoop. Unreachable today only by ordering — workers are always created from an already-initialized main runtime — so it becomes reachable with concurrent bootstrap (embedding host, reloadApplication). Wants std::call_once around the whole init-and-elect sequence plus a separate ready signal for workers. (Raised in review of fix: isolate/runtime lifetime — worker startup race, teardown use-after-free, and leaks #2006.)

• Partial native initialization is never unwound. If PrepareV8Runtime throws after Isolate::New(), the isolate is already in s_isolate2RuntimesCache but the Java-side rollback only unwinds Java state, leaving the isolate and the half-built Runtime allocated. The fix should reuse the two-window teardown above rather than add a third cleanup path. (Raised in review of fix: isolate/runtime lifetime — worker startup race, teardown use-after-free, and leaks #2006.)

Leaks still outstanding

• NativeScriptException::m_javascriptException (NativeScriptException.h:166, allocated :48-49) — a Persistent<Value>* holding a strong handle to the JS Error and its captured stack. This is the only one that grows within a live runtime, including the long-lived main one, once per NativeScriptException(TryCatch&) that does not round-trip through Java. Not a simple destructor fix: the raw pointer is deliberately handed to Java as a jlong (NativeScriptException.cpp:152 → NativeScriptException.java:6) and reclaimed at :803-810, so a naive destructor double-frees. Fixing it means changing that cross-language ownership contract.

• ModuleInternal::m_loadedModules (ModuleInternal.h:101) — one Persistent<Object> per loaded module (ModuleInternal.cpp:376, :570), never freed; ~ModuleInternal frees its three siblings but not this one. Small (bounded by module count). Trap: TempModule inserts the same pointer under two keys (m_modulePath and m_cacheKey), so a naive delete-every-value loop double-frees — dedup by pointer.

• CtorCacheData::instanceMethodCallbacks — the MethodCallbackData* elements (MetadataNode.cpp:680, :706, :856, :899, :2217), each carrying a vector<MetadataEntry> (~200-500 B). fix: release ObjectManager's JS handles and JNI weak refs at teardown #2008 frees the two Persistents in those caches but deliberately leaves these: they may be shared across entries, so it needs an ownership analysis first.

• PODs attached to V8 objects with no finalizer at all — TypeMetadata (MetadataNode.cpp:1104, :1776), FieldCallbackData (:757, :874, :922), PropertyCallbackData (:787), ExtendedClassCallbackData (:1757, which also holds a strong Persistent<Object> pinning the whole JS implementation object). These leak on every GC, not only at teardown. Note MetadataNode.cpp contains no delete at all. They want an owner on the per-runtime MetadataNodeCache.

Cross-isolate sharing (not leaks — the same failure class as #2006)

• HMRSupport's three global maps (HMRSupport.cpp:22-24) — g_hotData, g_hotAccept, g_hotDispose, path-keyed, process-wide, unsynchronized, never cleaned, reached from InitializeImportMetaObject for every ES module. Same shape as the ES-module registry and the same .mjs gating, so low probability today. Should move to RuntimeState.

• MetadataNode's three static node caches (MetadataNode.cpp:2315-2317) — s_name2NodeCache, s_name2TreeNodeCache, s_treeNode2NodeCache, still unguarded and mutated from any runtime's thread (GetOrCreate inserts at :144-151, on every JS-wrapper creation via ObjectManager::CreateJSWrapperHelper). Not a leak — the nodes are bounded — but it is the same container-corruption shape as the Console crash, on a hotter path.

• The metadata tree and MetadataReader's buffers. Bigger than the caches: m_valueData/m_valueLength (MetadataReader.cpp:320-323) is a bump allocator, and m_v.push_back (:219, :264, :328) reallocates a vector that GetNodeById (:96-98) indexes with no bounds check — a realloc under a concurrent read is a use-after-free. Only reached for types absent from the static metadata, so rare but destructive.

  This one cannot take a coarse lock. GetOrCreateTreeNodeByName mutates those buffers while calling back into Java (CallbackHandlers::GetTypeMetadata → Runtime.getTypeMetadata → Class.forName). A function-scope mutex would be held across ART class loading and, on the .extend() path, dex generation — and could invert against the process-wide monitor in Runtime.java:229 and the r.wait() cross-thread callJSMethod path. It needs a narrow recursive mutex guarding state access only, with the ordering rule: the only permitted successor is Runtime::s_runtimeCacheMutex; never hold it across a JNI call that can re-enter JS.

Cosmetic

• ODR violation in IsolateDisposer.h:24-25 — isolateBoundObjects_ and isolateBoundObjectsMutex_ are defined at namespace scope in a header included by three TUs. nm shows the same mangled symbol as a strong definition in each object file; the link collapses them, so it is benign in practice and the map genuinely is shared as intended, but it should be inline (C++17).

---

Already handled elsewhere, recorded so they are not re-investigated

• g_moduleRegistry — the process-wide, unsynchronized, path-keyed ES-module registry is reworked into per-isolate state in feat: ESM resolver hardening, HTTP module loader, ns:module dev surface #1965.
• SIG_handler throwing from a signal handler (which suppressed tombstones and is why all of this read as flaky tests for weeks) — removed in fix: restore native tombstones and record runtime crash breadcrumbs #2007.

Verified not leaks

Timers, BuiltinLoader, NsBuiltinModules, Events/ErrorEvents/PromiseRejectionTracker, WorkerInspectorClient, the napi layer, DirectBuffer, and WeakRef all release correctly, mostly via registerIsolateBoundObject or explicit resets in DestroyRuntime before disposal. JsV8InspectorClient::Domains looked like cross-isolate corruption but is not: only the main isolate ever populates it (__registerDomainDispatcher is installed by registerModules() alone, and WorkerInspectorClient never installs it). Cached jclass statics look like dangling local refs but come from JEnv::FindClass, which returns process-wide global refs.