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();