From 69f6e84e2bfd5815547b8b80042266e90ed1ceeb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Branimir=20Karad=C5=BEi=C4=87?= Date: Tue, 6 Oct 2026 14:19:04 -0700 Subject: [PATCH 1/3] Release Hermes WeakRef targets after draining jobs Clear the runtime's kept-objects set after its microtask queue drains, matching the Hermes JSI and ABI embeddings. Otherwise WeakRef targets remain runtime roots across turns and retain disposed application graphs. Add a bounded native regression that retains 256 MiB without the fix and less than 64 MiB with it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 60c2ec68-6de1-445d-9fc9-b699db737eae --- Core/Node-API/Source/env_hermes.cc | 2 ++ Tests/UnitTests/Source/Tests.NodeApi.cpp | 37 ++++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/Core/Node-API/Source/env_hermes.cc b/Core/Node-API/Source/env_hermes.cc index c3a97db0..1db67572 100644 --- a/Core/Node-API/Source/env_hermes.cc +++ b/Core/Node-API/Source/env_hermes.cc @@ -164,6 +164,8 @@ namespace Napi // exceptions thrown FROM user callbacks are already handled in // AppRuntime::Dispatch's try/catch. (void)runtime->drainJobs(); + // WeakRef targets are kept alive only until the current job finishes. + runtime->clearKeptObjects(); } Napi::Value Eval(Napi::Env env, const char* source, const char* sourceUrl) diff --git a/Tests/UnitTests/Source/Tests.NodeApi.cpp b/Tests/UnitTests/Source/Tests.NodeApi.cpp index 69d88deb..e2ad52d7 100644 --- a/Tests/UnitTests/Source/Tests.NodeApi.cpp +++ b/Tests/UnitTests/Source/Tests.NodeApi.cpp @@ -1,10 +1,47 @@ #include #include +#include +#include #include #include +#include #include #include +TEST(NodeApi, HermesDrainJobsReleasesWeakRefTargets) +{ + if (std::string_view{NAPI_JAVASCRIPT_ENGINE} != "Hermes") + { + GTEST_SKIP() << "Uses Hermes heap statistics."; + } + + std::promise externalBytes; + auto completed = externalBytes.get_future(); + Babylon::AppRuntime runtime{}; + runtime.Dispatch([&](Napi::Env env) { + try + { + for (size_t i = 0; i < 256; ++i) + { + { + Napi::HandleScope scope{env}; + Napi::Eval(env, "new WeakRef(new Uint8Array(1024 * 1024));", "weakref-job.js"); + } + Napi::DrainJobs(env); + } + externalBytes.set_value(Napi::Eval(env, + "HermesInternal.getInstrumentedStats().js_externalBytes", + "weakref-heap.js").As().DoubleValue()); + } + catch (const Napi::Error& error) + { + externalBytes.set_exception(std::make_exception_ptr(std::runtime_error{error.Message()})); + } + }); + ASSERT_EQ(completed.wait_for(std::chrono::seconds{30}), std::future_status::ready); + EXPECT_LT(completed.get(), 64 * 1024 * 1024); +} + // The V8JSI Node-API shim does not implement napi_create_dataview / // napi_get_dataview_info (its DataView::New throws "TODO"), so this native test // only builds on the Chakra, V8, and JavaScriptCore backends. The size_t-width From 11e645fa8336e4b811110fbc535542d181fd3845 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Branimir=20Karad=C5=BEi=C4=87?= Date: Tue, 6 Oct 2026 15:17:44 -0700 Subject: [PATCH 2/3] Build the WeakRef regression only for Hermes Move the test into the existing engine-specific source selection. A runtime skip cannot hide the Hermes-only DrainJobs API from other engines' compilers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 60c2ec68-6de1-445d-9fc9-b699db737eae --- Tests/UnitTests/CMakeLists.txt | 2 + .../UnitTests/Source/Tests.NodeApi.Hermes.cpp | 36 ++++++++++++++++++ Tests/UnitTests/Source/Tests.NodeApi.cpp | 37 ------------------- 3 files changed, 38 insertions(+), 37 deletions(-) create mode 100644 Tests/UnitTests/Source/Tests.NodeApi.Hermes.cpp diff --git a/Tests/UnitTests/CMakeLists.txt b/Tests/UnitTests/CMakeLists.txt index 6f6f88a6..4171ff13 100644 --- a/Tests/UnitTests/CMakeLists.txt +++ b/Tests/UnitTests/CMakeLists.txt @@ -65,6 +65,8 @@ endif() if(NAPI_JAVASCRIPT_ENGINE STREQUAL "Chakra") set(SOURCES ${SOURCES} "Source/Tests.NodeApi.Chakra.cpp") +elseif(NAPI_JAVASCRIPT_ENGINE STREQUAL "Hermes") + set(SOURCES ${SOURCES} "Source/Tests.NodeApi.Hermes.cpp") elseif(NAPI_JAVASCRIPT_ENGINE STREQUAL "JavaScriptCore") set(SOURCES ${SOURCES} "Source/Tests.NodeApi.JavaScriptCore.cpp") endif() diff --git a/Tests/UnitTests/Source/Tests.NodeApi.Hermes.cpp b/Tests/UnitTests/Source/Tests.NodeApi.Hermes.cpp new file mode 100644 index 00000000..820f8473 --- /dev/null +++ b/Tests/UnitTests/Source/Tests.NodeApi.Hermes.cpp @@ -0,0 +1,36 @@ +#include +#include +#include + +#include +#include +#include + +TEST(NodeApi, HermesDrainJobsReleasesWeakRefTargets) +{ + std::promise externalBytes; + auto completed = externalBytes.get_future(); + Babylon::AppRuntime runtime{}; + runtime.Dispatch([&](Napi::Env env) { + try + { + for (size_t i = 0; i < 256; ++i) + { + { + Napi::HandleScope scope{env}; + Napi::Eval(env, "new WeakRef(new Uint8Array(1024 * 1024));", "weakref-job.js"); + } + Napi::DrainJobs(env); + } + externalBytes.set_value(Napi::Eval(env, + "HermesInternal.getInstrumentedStats().js_externalBytes", + "weakref-heap.js").As().DoubleValue()); + } + catch (const Napi::Error& error) + { + externalBytes.set_exception(std::make_exception_ptr(std::runtime_error{error.Message()})); + } + }); + ASSERT_EQ(completed.wait_for(std::chrono::seconds{30}), std::future_status::ready); + EXPECT_LT(completed.get(), 64 * 1024 * 1024); +} diff --git a/Tests/UnitTests/Source/Tests.NodeApi.cpp b/Tests/UnitTests/Source/Tests.NodeApi.cpp index e2ad52d7..69d88deb 100644 --- a/Tests/UnitTests/Source/Tests.NodeApi.cpp +++ b/Tests/UnitTests/Source/Tests.NodeApi.cpp @@ -1,47 +1,10 @@ #include #include -#include -#include #include #include -#include #include #include -TEST(NodeApi, HermesDrainJobsReleasesWeakRefTargets) -{ - if (std::string_view{NAPI_JAVASCRIPT_ENGINE} != "Hermes") - { - GTEST_SKIP() << "Uses Hermes heap statistics."; - } - - std::promise externalBytes; - auto completed = externalBytes.get_future(); - Babylon::AppRuntime runtime{}; - runtime.Dispatch([&](Napi::Env env) { - try - { - for (size_t i = 0; i < 256; ++i) - { - { - Napi::HandleScope scope{env}; - Napi::Eval(env, "new WeakRef(new Uint8Array(1024 * 1024));", "weakref-job.js"); - } - Napi::DrainJobs(env); - } - externalBytes.set_value(Napi::Eval(env, - "HermesInternal.getInstrumentedStats().js_externalBytes", - "weakref-heap.js").As().DoubleValue()); - } - catch (const Napi::Error& error) - { - externalBytes.set_exception(std::make_exception_ptr(std::runtime_error{error.Message()})); - } - }); - ASSERT_EQ(completed.wait_for(std::chrono::seconds{30}), std::future_status::ready); - EXPECT_LT(completed.get(), 64 * 1024 * 1024); -} - // The V8JSI Node-API shim does not implement napi_create_dataview / // napi_get_dataview_info (its DataView::New throws "TODO"), so this native test // only builds on the Chakra, V8, and JavaScriptCore backends. The size_t-width From d28964faf4b56f48272f81baebc81722c8689987 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Branimir=20Karad=C5=BEi=C4=87?= Date: Tue, 6 Oct 2026 15:28:32 -0700 Subject: [PATCH 3/3] Measure scheduling test delays with the high-resolution clock Use performance.now() instead of Date timestamps for elapsed timer checks. This avoids x86 Chakra's wall-clock precision causing a 29 ms observation against a 30 ms interval bound. Keep all existing minimum delays unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 60c2ec68-6de1-445d-9fc9-b699db737eae --- .../UnitTests/Source/Scripts/tests.scheduling.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Tests/UnitTests/Source/Scripts/tests.scheduling.ts b/Tests/UnitTests/Source/Scripts/tests.scheduling.ts index 8b813f10..4d1cef55 100644 --- a/Tests/UnitTests/Source/Scripts/tests.scheduling.ts +++ b/Tests/UnitTests/Source/Scripts/tests.scheduling.ts @@ -14,10 +14,10 @@ describe("setTimeout", function () { }); it("should call the given function after the given delay", function (done) { - const startTime = new Date().getTime(); + const startTime = performance.now(); setTimeout(() => { try { - expect(new Date().getTime() - startTime).to.be.at.least(10); + expect(performance.now() - startTime).to.be.at.least(10); done(); } catch (e) { @@ -27,11 +27,11 @@ describe("setTimeout", function () { }); it("should call the given nested function after the given delay", function (done) { - const startTime = new Date().getTime(); + const startTime = performance.now(); setTimeout(() => { setTimeout(() => { try { - expect(new Date().getTime() - startTime).to.be.at.least(20); + expect(performance.now() - startTime).to.be.at.least(20); done(); } catch (e) { @@ -42,10 +42,10 @@ describe("setTimeout", function () { }); it("should call the given function after the given delay when the delay is a string representing a valid number", function (done) { - const startTime = new Date().getTime(); + const startTime = performance.now(); setTimeout(() => { try { - expect(new Date().getTime() - startTime).to.be.at.least(10); + expect(performance.now() - startTime).to.be.at.least(10); done(); } catch (e) { @@ -143,12 +143,12 @@ describe("setInterval", function () { }); it("should call the given function at the given interval", function (done) { - let startTime = new Date().getTime(); + const startTime = performance.now(); let tickCount = 0; const id = setInterval(() => { try { tickCount++; - expect(new Date().getTime() - startTime).to.be.at.least(tickCount * 10); + expect(performance.now() - startTime).to.be.at.least(tickCount * 10); if (tickCount > 2) { clearInterval(id); done();