From 8fd788ec77b51d64d6240984bfba737bd60b6f35 Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Tue, 25 Aug 2026 16:35:50 -0300 Subject: [PATCH] fix(runtime): drain autoreleased objects on worker threads per callout Worker threads run a bare CFRunLoopRun with no autorelease pool management, so autoreleased ObjC objects created while executing JS on a worker (marshalled returns, framework temporaries, call-scoped block copies) accumulated in the thread's bottom pool and only drained when the worker died - a leak proportional to worker lifetime and native-call volume. The main thread does not have this problem because UIKit drains a pool once per run-loop pass. Scope pools to the runtime's own callouts instead of run-loop passes: EventLoop::RunGuarded wraps each non-bare work unit (both scheduler lanes, where all worker JS executes; bare entries may @throw on purpose and stay unwrapped), the worker's message-drain source wraps DrainPendingTasks, and explicit pools cover the boot phase (runs before the loop) and teardown (otherwise drains only when the backing NSOperation ends). On the main thread this only tightens drain latency; the lifetime contract - alive at least to the end of the current callout - is unchanged. A UIKit-style run-loop observer (tried in both single-pool and pool-per-nesting-level forms) aborts with AutoreleasePoolPage::badPop under the HTTP-ESM loader tests: nested CFRunLoopRunInMode pumps interleave foreign pool lifetimes across callouts, cutting observer-owned tokens. A pool pushed and popped inside a single callout cannot be interleaved with, and matches the codebase's existing entry-point pattern. The new spec autoreleases a TNSAllocLog on the worker in one timer callout and reports TNSGetOutput() from the next; the dealloc entry can only be present in between if the pool drained per callout. --- NativeScript/runtime/EventLoop.mm | 8 ++- NativeScript/runtime/WorkerWrapper.mm | 49 ++++++++++++------- TestFixtures/Marshalling/TNSAllocLog.h | 4 ++ TestFixtures/Marshalling/TNSAllocLog.m | 6 +++ .../app/tests/WorkerAutoreleasePoolTests.js | 11 +++++ .../app/tests/autoreleasePoolDrainWorker.js | 11 +++++ TestRunner/app/tests/index.js | 2 + 7 files changed, 71 insertions(+), 20 deletions(-) create mode 100644 TestRunner/app/tests/WorkerAutoreleasePoolTests.js create mode 100644 TestRunner/app/tests/autoreleasePoolDrainWorker.js diff --git a/NativeScript/runtime/EventLoop.mm b/NativeScript/runtime/EventLoop.mm index 57aa131b..e91b3196 100644 --- a/NativeScript/runtime/EventLoop.mm +++ b/NativeScript/runtime/EventLoop.mm @@ -20,10 +20,16 @@ // a CFRunLoop callback frame. Deliberately no catch(...): on Darwin it would // also swallow NSExceptions, and bare entries (which may @throw on purpose) // never come through here anyway. +// The pool scopes autoreleased objects to the callout: worker threads have no +// UIKit observer draining once per run-loop pass, so without it they would +// accumulate until the worker dies. Bare entries stay unwrapped - an +// @throw must not unwind through a pool this code owns. template void RunGuarded(F&& body) { try { - body(); + @autoreleasepool { + body(); + } } catch (tns::NativeScriptException& ex) { Log(@"NativeScript: uncaught NativeScriptException in event loop task: %s", ex.getMessage().c_str()); diff --git a/NativeScript/runtime/WorkerWrapper.mm b/NativeScript/runtime/WorkerWrapper.mm index ff5a44e3..9a135117 100644 --- a/NativeScript/runtime/WorkerWrapper.mm +++ b/NativeScript/runtime/WorkerWrapper.mm @@ -147,13 +147,22 @@ static void PostToRuntimeLoop(Runtime* runtime, std::function fn, bool a runLoop, [](void* info) { WorkerWrapper* w = static_cast(info); - w->DrainPendingTasks(); + // Autoreleased objects die with the callout; a worker has no UIKit + // observer draining a pool per run-loop pass. + @autoreleasepool { + w->DrainPendingTasks(); + } }, this); - this->workerIsolate_ = func(); + // Autoreleased objects created during the boot phase (isolate creation and + // entry-script evaluation run before the loop starts) otherwise accumulate + // until the worker dies - the backing NSOperation's pool is the only drain. + @autoreleasepool { + this->workerIsolate_ = func(); - this->DrainPendingTasks(); + this->DrainPendingTasks(); + } // check again as it could terminate before this if (!this->isTerminating_) { @@ -161,22 +170,24 @@ static void PostToRuntimeLoop(Runtime* runtime, std::function fn, bool a } } - // The inspector must be gone before the Runtime (and with it the isolate) - // is deleted below. - this->DestroyInspector(); - - this->isDisposed_ = true; - Runtime* runtime = Runtime::GetCurrentRuntime(); - if (runtime != nullptr) { - delete runtime; - } else { - // Runtime was never created (worker terminated before initialization). - // The runtime destructor normally handles this cleanup, so do it here. - int workerId = this->workerId_; - bool found; - auto state = Caches::Workers->Get(workerId, found); - if (found) { - Caches::Workers->Remove(workerId); + @autoreleasepool { // teardown garbage drains before the thread is gone + // The inspector must be gone before the Runtime (and with it the isolate) + // is deleted below. + this->DestroyInspector(); + + this->isDisposed_ = true; + Runtime* runtime = Runtime::GetCurrentRuntime(); + if (runtime != nullptr) { + delete runtime; + } else { + // Runtime was never created (worker terminated before initialization). + // The runtime destructor normally handles this cleanup, so do it here. + int workerId = this->workerId_; + bool found; + auto state = Caches::Workers->Get(workerId, found); + if (found) { + Caches::Workers->Remove(workerId); + } } } } diff --git a/TestFixtures/Marshalling/TNSAllocLog.h b/TestFixtures/Marshalling/TNSAllocLog.h index 78c26378..8dcb1dcf 100644 --- a/TestFixtures/Marshalling/TNSAllocLog.h +++ b/TestFixtures/Marshalling/TNSAllocLog.h @@ -10,6 +10,10 @@ - (instancetype)init; - (void)dealloc; +// Creates an instance whose only reference is in the current autorelease pool, +// so its dealloc log marks when that pool drains. ++ (void)autoreleaseInstance; + @end #endif /* TNSAllocLog_h */ diff --git a/TestFixtures/Marshalling/TNSAllocLog.m b/TestFixtures/Marshalling/TNSAllocLog.m index 448da23a..f6c1cb64 100644 --- a/TestFixtures/Marshalling/TNSAllocLog.m +++ b/TestFixtures/Marshalling/TNSAllocLog.m @@ -20,4 +20,10 @@ - (void)dealloc { TNSLog(@"TNSAllocLog dealloc"); } ++ (void)autoreleaseInstance { + // CFBridgingRetain moves the instance's ownership out of ARC so the + // CFAutorelease'd reference in the current pool is the only one left. + CFAutorelease(CFBridgingRetain([[TNSAllocLog alloc] init])); +} + @end diff --git a/TestRunner/app/tests/WorkerAutoreleasePoolTests.js b/TestRunner/app/tests/WorkerAutoreleasePoolTests.js new file mode 100644 index 00000000..a77356ce --- /dev/null +++ b/TestRunner/app/tests/WorkerAutoreleasePoolTests.js @@ -0,0 +1,11 @@ +describe("worker autorelease pools", function () { + it("drains autoreleased objects per event-loop callout, not at worker death", function (done) { + var worker = new Worker("~/tests/autoreleasePoolDrainWorker.js"); + worker.onmessage = function (e) { + worker.terminate(); + expect(e.data).toContain("TNSAllocLog init"); + expect(e.data).toContain("TNSAllocLog dealloc"); + done(); + }; + }); +}); diff --git a/TestRunner/app/tests/autoreleasePoolDrainWorker.js b/TestRunner/app/tests/autoreleasePoolDrainWorker.js new file mode 100644 index 00000000..8d14b4b2 --- /dev/null +++ b/TestRunner/app/tests/autoreleasePoolDrainWorker.js @@ -0,0 +1,11 @@ +// The autorelease happens inside one timer callout, and the report is sent +// from the NEXT one: the dealloc log can only be present in between if the +// worker drains a pool per callout. Without that the pool only drains at +// worker death, after the report is sent. +TNSClearOutput(); +setTimeout(function () { + TNSAllocLog.autoreleaseInstance(); + setTimeout(function () { + postMessage(TNSGetOutput()); + }, 0); +}, 0); diff --git a/TestRunner/app/tests/index.js b/TestRunner/app/tests/index.js index 4b6558f6..0ccccd23 100644 --- a/TestRunner/app/tests/index.js +++ b/TestRunner/app/tests/index.js @@ -192,6 +192,8 @@ require("./NapiCoverageTests"); // Worker-isolate scoping of extended objc class names require("./ExtendedClassNamingTests"); +require("./WorkerAutoreleasePoolTests"); + // Tests common for all runtimes (git submodule of NativeScript/common-runtime-tests-app). require("../shared/index").runAllTests();