From 6989c6549b9a3639d56165c14aa4cd64407915c8 Mon Sep 17 00:00:00 2001 From: Cedric Guillemet <1312968+CedricGuillemet@users.noreply.github.com> Date: Wed, 3 Jun 2026 14:18:29 +0200 Subject: [PATCH 01/20] Hermes + Napi --- .github/workflows/build-macos.yml | 16 +- .github/workflows/build-win32.yml | 5 +- .github/workflows/ci.yml | 17 ++ CMakeLists.txt | 35 +++ Core/AppRuntime/Include/Babylon/AppRuntime.h | 9 + Core/AppRuntime/Source/AppRuntime.cpp | 14 ++ Core/AppRuntime/Source/AppRuntime_Chakra.cpp | 7 + Core/AppRuntime/Source/AppRuntime_Hermes.cpp | 28 +++ Core/AppRuntime/Source/AppRuntime_JSI.cpp | 5 + .../Source/AppRuntime_JavaScriptCore.cpp | 5 + Core/AppRuntime/Source/AppRuntime_V8.cpp | 6 + Core/Node-API/CMakeLists.txt | 94 +++++++- .../Node-API/Include/Engine/Hermes/napi/env.h | 33 +++ Core/Node-API/Source/env_hermes.cc | 223 ++++++++++++++++++ 14 files changed, 492 insertions(+), 5 deletions(-) create mode 100644 Core/AppRuntime/Source/AppRuntime_Hermes.cpp create mode 100644 Core/Node-API/Include/Engine/Hermes/napi/env.h create mode 100644 Core/Node-API/Source/env_hermes.cc diff --git a/.github/workflows/build-macos.yml b/.github/workflows/build-macos.yml index 24a6af91..0faaf938 100644 --- a/.github/workflows/build-macos.yml +++ b/.github/workflows/build-macos.yml @@ -18,11 +18,22 @@ on: required: false type: boolean default: false + js-engine: + # When unset we let Core/Node-API/CMakeLists.txt pick the per-platform + # default (JavaScriptCore on macOS). Pass "Hermes" to fetch and link + # against facebook/hermes (static_h branch); pass "V8"/"Chakra"/etc. + # to override. + required: false + type: string + default: '' jobs: build: runs-on: ${{ inputs.runs-on }} - timeout-minutes: 15 + # Hermes builds take roughly 4x as long as a JSC build because the + # umbrella `hermesvm_a` static lib drags in the full Hermes compiler + # pipeline. Allow a wider window when Hermes is selected. + timeout-minutes: ${{ inputs.js-engine == 'Hermes' && 60 || 15 }} steps: - uses: actions/checkout@v5 @@ -33,7 +44,8 @@ jobs: run: | cmake -B Build/macOS -G Xcode \ -D ENABLE_SANITIZERS=${{ inputs.enable-sanitizers && 'ON' || 'OFF' }} \ - -D ENABLE_THREAD_SANITIZER=${{ inputs.enable-thread-sanitizer && 'ON' || 'OFF' }} + -D ENABLE_THREAD_SANITIZER=${{ inputs.enable-thread-sanitizer && 'ON' || 'OFF' }} \ + ${{ inputs.js-engine != '' && format('-D NAPI_JAVASCRIPT_ENGINE={0}', inputs.js-engine) || '' }} - name: Build run: cmake --build Build/macOS --target UnitTests --config RelWithDebInfo diff --git a/.github/workflows/build-win32.yml b/.github/workflows/build-win32.yml index 4fa03e85..1c2caab3 100644 --- a/.github/workflows/build-win32.yml +++ b/.github/workflows/build-win32.yml @@ -13,7 +13,10 @@ on: jobs: build: runs-on: windows-latest - timeout-minutes: 15 + # Hermes pulls in a large LLVH/Boost.Context/BCGen chain through + # `hermesvm_a`; on `windows-latest` the full configure+build comfortably + # exceeds the standard 15-minute window. Keep other engines snappy. + timeout-minutes: ${{ inputs.js-engine == 'Hermes' && 60 || 15 }} steps: - uses: actions/checkout@v5 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 752cda09..d7b3e07d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,6 +32,12 @@ jobs: platform: x64 js-engine: V8 + Win32_x64_Hermes: + uses: ./.github/workflows/build-win32.yml + with: + platform: x64 + js-engine: Hermes + # ── UWP ─────────────────────────────────────────────────────── UWP_x64_Chakra: uses: ./.github/workflows/build-uwp.yml @@ -109,6 +115,17 @@ jobs: runs-on: macos-26 enable-thread-sanitizer: true + # Hermes on macOS: only one variant — Hermes is expensive to build, and + # we already cover both Xcode toolchains for JSC. Pinning to 26.4 keeps + # the matrix on the newer toolchain that downstream embedders care about + # most; bump if needed. + macOS_Xcode264_Hermes: + uses: ./.github/workflows/build-macos.yml + with: + xcode-version: '26.4' + runs-on: macos-26 + js-engine: Hermes + # ── iOS ─────────────────────────────────────────────────────── iOS_Xcode164: uses: ./.github/workflows/build-ios.yml diff --git a/CMakeLists.txt b/CMakeLists.txt index 606b5bc1..4acb759d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -28,6 +28,14 @@ FetchContent_Declare(CMakeExtensions FetchContent_Declare(googletest URL "https://github.com/google/googletest/archive/refs/tags/v1.17.0.tar.gz" EXCLUDE_FROM_ALL) +FetchContent_Declare(hermes + GIT_REPOSITORY https://github.com/facebook/hermes.git + # Pinned to the tip of the `static_h` branch as of 2026-06-03 so CI is + # reproducible. Bump this SHA when you intentionally want to pick up + # upstream Hermes changes — keep it on the static_h branch (Static + # Hermes is the variant our NAPI integration targets). + GIT_TAG 348582831f50954895da8e80cc91112d51036c69 + EXCLUDE_FROM_ALL) FetchContent_Declare(ios-cmake GIT_REPOSITORY https://github.com/leetal/ios-cmake.git GIT_TAG 4.4.1 @@ -144,6 +152,33 @@ if(BABYLON_DEBUG_TRACE) add_definitions(-DBABYLON_DEBUG_TRACE) endif() +if(NAPI_JAVASCRIPT_ENGINE STREQUAL "Hermes") + # Configure Hermes static_h options BEFORE making it available so that + # cache variables take effect. We disable the parts of Hermes we don't + # need (the unit-test suite, debugger) to keep build time reasonable and + # to avoid pulling extra targets into our build tree. Notably we leave + # HERMES_ENABLE_NAPI ON (the default) — that's the whole point of + # integrating Hermes here. + # + # HERMES_ENABLE_TOOLS must stay ON: even when HERMES_ENABLE_TEST_SUITE is + # OFF, Hermes unconditionally adds external/node-api-cts and + # external/node-api-tests whenever HERMES_ENABLE_NAPI is on, and those + # subdirectories reference the `hermes` CLI tool target via + # $. CMake fails to generate if the target doesn't + # exist, so we let Hermes build its tools. None of them ship in our + # final binaries because they're EXCLUDE_FROM_ALL via FetchContent. + set(HERMES_ENABLE_TOOLS ON CACHE BOOL "" FORCE) + set(HERMES_ENABLE_TEST_SUITE OFF CACHE BOOL "" FORCE) + set(HERMES_ENABLE_DEBUGGER OFF CACHE BOOL "" FORCE) + set(HERMES_BUILD_APPLE_FRAMEWORK OFF CACHE BOOL "" FORCE) + set(HERMES_ENABLE_NAPI ON CACHE BOOL "" FORCE) + # Hermes ships its own bundled gtest as `llvh-gtest` (a different + # CMake target name from googletest's `gtest`), so the two coexist + # without clashing. We never link llvh-gtest into our UnitTests + # executable, so there's no duplicate-symbol issue at link time. + FetchContent_MakeAvailable_With_Message(hermes) +endif() + if(NAPI_JAVASCRIPT_ENGINE STREQUAL "V8" AND JSRUNTIMEHOST_CORE_APPRUNTIME_V8_INSPECTOR) FetchContent_MakeAvailable_With_Message(asio) add_library(asio INTERFACE) diff --git a/Core/AppRuntime/Include/Babylon/AppRuntime.h b/Core/AppRuntime/Include/Babylon/AppRuntime.h index f6ca7dfd..f690ce11 100644 --- a/Core/AppRuntime/Include/Babylon/AppRuntime.h +++ b/Core/AppRuntime/Include/Babylon/AppRuntime.h @@ -67,6 +67,15 @@ namespace Babylon // extra logic around the invocation of a dispatched callback. void Execute(Dispatchable callback); + // Engine-specific hook called from Dispatch immediately after a user + // callback completes. Most engines auto-drain microtasks at scope + // exit, so the implementation is a no-op for Chakra/V8/JSC/JSI. + // Hermes does NOT auto-drain; its implementation calls + // `Napi::DrainJobs(env)` so Promise continuations and queueMicrotask + // callbacks scheduled during the user callback actually run before + // the next top-level dispatch. + void DrainMicrotasks(Napi::Env env); + Options m_options; class Impl; diff --git a/Core/AppRuntime/Source/AppRuntime.cpp b/Core/AppRuntime/Source/AppRuntime.cpp index 99298df2..176bc849 100644 --- a/Core/AppRuntime/Source/AppRuntime.cpp +++ b/Core/AppRuntime/Source/AppRuntime.cpp @@ -109,6 +109,13 @@ namespace Babylon { m_impl->Append([this, func{std::move(func)}](Napi::Env env) mutable { Execute([this, env, func{std::move(func)}]() mutable { + // Some engines (notably Hermes) require an open NAPI handle + // scope before any napi_* call that materializes a value. + // The other engines (V8/Chakra/JSC) already provide an outer + // scope at the RunEnvironmentTier level, so this extra + // scope is harmless there but mandatory for Hermes. + Napi::HandleScope scope{env}; + try { func(env); @@ -122,6 +129,13 @@ namespace Babylon assert(false); std::abort(); } + + // Drain engine-level microtasks/jobs queued during the + // callback (Promise continuations, queueMicrotask, etc.) so + // they run before the next top-level Dispatch. No-op for + // engines that drain automatically; Hermes needs an explicit + // pump. + DrainMicrotasks(env); }); }); } diff --git a/Core/AppRuntime/Source/AppRuntime_Chakra.cpp b/Core/AppRuntime/Source/AppRuntime_Chakra.cpp index 2329ad30..315954c2 100644 --- a/Core/AppRuntime/Source/AppRuntime_Chakra.cpp +++ b/Core/AppRuntime/Source/AppRuntime_Chakra.cpp @@ -71,4 +71,11 @@ namespace Babylon // Detach must come after JsDisposeRuntime since it triggers finalizers which require env. Napi::Detach(env); } + + void AppRuntime::DrainMicrotasks(Napi::Env) + { + // Chakra drains promise continuations through its + // JsSetPromiseContinuationCallback hook (see RunEnvironmentTier). + // No explicit pump needed here. + } } diff --git a/Core/AppRuntime/Source/AppRuntime_Hermes.cpp b/Core/AppRuntime/Source/AppRuntime_Hermes.cpp new file mode 100644 index 00000000..12debfcb --- /dev/null +++ b/Core/AppRuntime/Source/AppRuntime_Hermes.cpp @@ -0,0 +1,28 @@ +#include "AppRuntime.h" +#include + +namespace Babylon +{ + void AppRuntime::RunEnvironmentTier(const char*) + { + // All Hermes runtime + napi_env setup is encapsulated inside the napi + // library's env_hermes.cc (see Napi::Attach/Detach). Keeping the + // engine-specific machinery there avoids dragging Hermes headers into + // AppRuntime's translation unit. + Napi::Env env = Napi::Attach(); + + Run(env); + + Napi::Detach(env); + } + + void AppRuntime::DrainMicrotasks(Napi::Env env) + { + // Hermes does not auto-drain its job queue. Promise continuations, + // queueMicrotask callbacks, and pending NAPI finalizers all run via + // Runtime::drainJobs(). We pump it after each user callback so async + // code (Promises, Mocha's async tests, polyfill schedulers, etc.) + // observes the same "between turns" semantics it gets on V8/Chakra. + Napi::DrainJobs(env); + } +} diff --git a/Core/AppRuntime/Source/AppRuntime_JSI.cpp b/Core/AppRuntime/Source/AppRuntime_JSI.cpp index 3ddd19da..a7c809c7 100644 --- a/Core/AppRuntime/Source/AppRuntime_JSI.cpp +++ b/Core/AppRuntime/Source/AppRuntime_JSI.cpp @@ -45,4 +45,9 @@ namespace Babylon Napi::Detach(env); } + + void AppRuntime::DrainMicrotasks(Napi::Env) + { + // JSI/V8 backed JSI auto-drains microtasks per scope. + } } diff --git a/Core/AppRuntime/Source/AppRuntime_JavaScriptCore.cpp b/Core/AppRuntime/Source/AppRuntime_JavaScriptCore.cpp index a0322898..b1334c22 100644 --- a/Core/AppRuntime/Source/AppRuntime_JavaScriptCore.cpp +++ b/Core/AppRuntime/Source/AppRuntime_JavaScriptCore.cpp @@ -23,4 +23,9 @@ namespace Babylon // Detach must come after JSGlobalContextRelease since it triggers finalizers which require env. Napi::Detach(env); } + + void AppRuntime::DrainMicrotasks(Napi::Env) + { + // JavaScriptCore drains microtasks automatically at script boundaries. + } } diff --git a/Core/AppRuntime/Source/AppRuntime_V8.cpp b/Core/AppRuntime/Source/AppRuntime_V8.cpp index 1297fdbb..89928dcf 100644 --- a/Core/AppRuntime/Source/AppRuntime_V8.cpp +++ b/Core/AppRuntime/Source/AppRuntime_V8.cpp @@ -112,4 +112,10 @@ namespace Babylon // delete isolate->GetArrayBufferAllocator(); isolate->Dispose(); } + + void AppRuntime::DrainMicrotasks(Napi::Env) + { + // V8 auto-drains microtasks at the end of each script/callback when + // using the default MicrotasksPolicy. No explicit pump needed. + } } diff --git a/Core/Node-API/CMakeLists.txt b/Core/Node-API/CMakeLists.txt index 84c808fa..6835326a 100644 --- a/Core/Node-API/CMakeLists.txt +++ b/Core/Node-API/CMakeLists.txt @@ -17,8 +17,16 @@ set(SOURCES "Include/Shared/napi/js_native_api.h" "Include/Shared/napi/js_native_api_types.h" "Include/Shared/napi/napi.h" - "Include/Shared/napi/napi-inl.h" - "Source/env.cc") + "Include/Shared/napi/napi-inl.h") + +# env.cc contains a generic `Napi::Eval` that goes through the C++ wrapper's +# `Env::RunScript`, which calls our 4-argument `napi_run_script` (Babylon +# extension carrying a `source_url`). Hermes only provides the standard +# 3-argument `napi_run_script`, so for Hermes we skip env.cc and provide a +# Hermes-native `Napi::Eval` in env_hermes.cc that calls `hermes_run_script`. +if(NOT NAPI_JAVASCRIPT_ENGINE STREQUAL "Hermes") + list(APPEND SOURCES "Source/env.cc") +endif() set(INCLUDE_DIRECTORIES PUBLIC "Include/Shared" @@ -145,6 +153,81 @@ if(NAPI_BUILD_ABI) else() message(FATAL_ERROR "Unsupported JavaScript engine: ${NAPI_JAVASCRIPT_ENGINE}") endif() + elseif(NAPI_JAVASCRIPT_ENGINE STREQUAL "Hermes") + # Hermes provides its own implementation of the standard Node-API C + # functions inside the `hermesNapi` static library, so we don't ship a + # `js_native_api_hermes.cc` here. We only contribute the Babylon-side + # bridge (`env_hermes.cc`) which adapts our `Napi::Attach`/`Detach`/ + # `Eval` helpers to Hermes's `hermes_napi_create_env` / + # `hermes_run_script` APIs. + set(SOURCES ${SOURCES} + "Source/env_hermes.cc") + + if(NOT TARGET hermesNapi) + message(FATAL_ERROR "NAPI_JAVASCRIPT_ENGINE=Hermes requires the hermesNapi target. \ +Make sure Hermes was fetched at the top-level CMakeLists.txt and NAPI_JAVASCRIPT_ENGINE was set BEFORE configure.") + endif() + + # Link Hermes PRIVATEly so its include directories and compile + # definitions (notably `NAPI_VERSION=10` and the vendored + # `hermes/napi/*` headers) don't leak to consumers of `napi`. + # Consumers continue to see only our shared `napi/*` headers. + # + # We link against Hermes's umbrella static library `hermesvm_a` (which + # transitively pulls in all the BCGen/Optimizer/Regex/Support/etc. + # object libraries that `hermesVMRuntime` depends on) plus the NAPI + # static library on top. Linking just `hermesNapi + hermesVMRuntime + + # hermesPublic` leaves a long tail of unresolved symbols + # (bigint::exponentiate, regex::parseRegex, hbc::compileEvalModule, + # llvh::raw_ostream, platform_unicode::normalize, ...) which + # `hermesvm_a` bundles together for us. + set(LINK_LIBRARIES ${LINK_LIBRARIES} + PRIVATE hermesNapi + PRIVATE hermesvm_a) + + # `hermes_napi.h` lives in Hermes's `API/napi/` source directory and + # is NOT installed alongside the other `include/hermes/napi/` + # headers, so the hermesNapi PUBLIC include dir doesn't expose it. + # Add the API/napi source directory explicitly so env_hermes.cc can + # `#include "hermes_napi.h"`. + # + # Additionally, Hermes uses *global* include_directories() at its + # project root (for the vendored LLVH headers and Hermes config + # output) instead of attaching them to each target. Those globals + # don't propagate out of Hermes's add_subdirectory scope, so any + # consumer (including our napi target) that pulls in Hermes headers + # transitively needs them re-declared here. + # + # We resolve the binary directory via CMAKE_BINARY_DIR rather than + # ${hermes_BINARY_DIR} because FetchContent's lowercased per-content + # binary variable isn't always populated reliably across CMake + # versions/scopes, whereas the standard `_deps/-build` layout + # is. + set(_HERMES_BINARY_DIR "${CMAKE_BINARY_DIR}/_deps/hermes-build") + set(INCLUDE_DIRECTORIES ${INCLUDE_DIRECTORIES} + PRIVATE "${hermes_SOURCE_DIR}/API/napi" + PRIVATE "${hermes_SOURCE_DIR}/include" + PRIVATE "${hermes_SOURCE_DIR}/public" + PRIVATE "${hermes_SOURCE_DIR}/external/llvh/include" + PRIVATE "${hermes_SOURCE_DIR}/external/llvh/gen/include" + PRIVATE "${_HERMES_BINARY_DIR}/include" + PRIVATE "${_HERMES_BINARY_DIR}/lib/config" + PRIVATE "${_HERMES_BINARY_DIR}/external/llvh/include") + + # Hermes's headers rely on a handful of MSVC warning suppressions + # that Hermes applies globally in its own `Hermes.cmake` (e.g. + # `-wd4576` for C-style compound literals like `(SHLegacyValue){...}` + # inside `sh_legacy_value.h`). Those globals don't propagate to our + # `napi` target, so we apply the minimum set needed for the headers + # we transitively pull in via `env_hermes.cc`. + set(NAPI_HERMES_COMPILE_OPTIONS + /wd4068 # unknown pragma (GCC pragmas in Hermes headers) + /wd4141 # 'inline': used more than once + /wd4146 # unary minus on unsigned type + /wd4244 # conversion possible loss of data + /wd4267 # size_t -> smaller type conversion + /wd4291 # no matching delete found + /wd4576) # parenthesized type followed by initializer list else() message(FATAL_ERROR "Unsupported JavaScript engine: ${NAPI_JAVASCRIPT_ENGINE}") endif() @@ -157,5 +240,12 @@ add_library(napi ${SOURCES}) target_include_directories(napi ${INCLUDE_DIRECTORIES}) target_link_libraries(napi ${LINK_LIBRARIES}) +if(NAPI_JAVASCRIPT_ENGINE STREQUAL "Hermes") + # Apply Hermes-specific warning suppressions ONLY to env_hermes.cc so + # they don't relax the rules for the rest of the napi sources. + set_source_files_properties("Source/env_hermes.cc" + PROPERTIES COMPILE_OPTIONS "${NAPI_HERMES_COMPILE_OPTIONS}") +endif() + set_property(TARGET napi PROPERTY FOLDER Dependencies) source_group(TREE ${CMAKE_CURRENT_SOURCE_DIR} FILES ${SOURCES}) diff --git a/Core/Node-API/Include/Engine/Hermes/napi/env.h b/Core/Node-API/Include/Engine/Hermes/napi/env.h new file mode 100644 index 00000000..ac806a4b --- /dev/null +++ b/Core/Node-API/Include/Engine/Hermes/napi/env.h @@ -0,0 +1,33 @@ +#pragma once + +#include + +namespace Napi +{ + // Create a Hermes runtime + napi_env owned by this process and expose it + // through Napi::Env. The runtime lives until the matching Detach() call. + Napi::Env Attach(); + + // Tear down the runtime that backs `env`. After this call, `env` is + // invalid. Hermes owns the env lifetime via its Runtime; destroying the + // runtime destroys the env, so we explicitly route Detach through a + // reverse lookup that drops the owning std::shared_ptr. + void Detach(Napi::Env env); + + // Compile and execute UTF-8 source on the current Hermes runtime. + // `sourceUrl` is attached to stack traces. Unlike the other engines + // we don't go through `Env::RunScript` because Hermes's standard + // `napi_run_script` is the canonical 3-argument signature, while the + // shared header carries a Babylon-specific 4-argument variant with a + // `source_url` parameter that Hermes doesn't provide. Instead we call + // Hermes's `hermes_run_script` directly inside the engine TU. + Napi::Value Eval(Napi::Env env, const char* source, const char* sourceUrl); + + // Pump Hermes's job queue (drains microtasks and pending finalizers). + // The application runtime must call this once per dispatched callback + // so that Promise continuations, queueMicrotask, and other deferred + // work scheduled during the callback actually runs before the next + // top-level dispatch. Equivalent engines (V8, Chakra) auto-drain + // microtasks at scope exit; Hermes requires an explicit drainJobs(). + void DrainJobs(Napi::Env env); +} diff --git a/Core/Node-API/Source/env_hermes.cc b/Core/Node-API/Source/env_hermes.cc new file mode 100644 index 00000000..9a58f710 --- /dev/null +++ b/Core/Node-API/Source/env_hermes.cc @@ -0,0 +1,223 @@ +// Bridge between the Babylon Napi:: helpers and the Hermes static_h NAPI +// implementation. +// +// Hermes's `hermesNapi` static library implements the full set of standard +// `napi_*` C functions on top of a `hermes::vm::Runtime`. In this TU we: +// 1. Create a Runtime + napi_env via `hermes_napi_create_env`. +// 2. Keep the Runtime alive for the lifetime of the env in a process-wide +// table (the env is opaque to callers; we need somewhere to stash the +// owning shared_ptr). +// 3. Provide `Napi::Eval` that calls Hermes's `hermes_run_script` (which +// takes a `source_url` for stack traces, matching what callers expect +// from the other engines' `Napi::Eval`). +// 4. Expose `Napi::DrainJobs` so AppRuntime can pump microtasks after each +// dispatched callback. +// +// Header layering note: +// Both our shared NAPI headers and Hermes's vendored ones use the same +// include guards (`SRC_JS_NATIVE_API_H_`, `SRC_JS_NATIVE_API_TYPES_H_`). +// Whichever set is included first wins. We deliberately include our shared +// `` chain FIRST so: +// * the Napi:: C++ wrappers (`Napi::Env`, `Napi::Value`, `Napi::Error`) +// line up with the rest of the project, +// * the Babylon-extended 4-arg `napi_run_script` declaration matches the +// inline `Env::RunScript` body in napi-inl.h (which we never actually +// call in this engine — Napi::Eval below routes through +// `hermes_run_script` directly — so the linker is never asked to find +// the 4-arg symbol). +// +// We keep `NAPI_VERSION` at the shared default (5) so the inline wrappers +// in napi-inl.h that target newer NAPI revisions (e.g. +// `Env::GetModuleFileName` which calls `node_api_get_module_file_name`) +// aren't pulled in — they would reference symbols absent from our shared +// js_native_api.h. +// +// Hermes's `node_api.h` then needs a couple of NAPI v10 types +// (`node_api_basic_env`, `node_api_basic_finalize`) that its own +// js_native_api.h would have defined, but those headers are now guarded +// out. We supply the typedefs locally: in NAPI v10 they're just type- +// attributed aliases for the regular `napi_env` / `napi_finalize`, so the +// ABI remains compatible with the symbols hermesNapi actually exports. + +#include + +// Forward-declare the NAPI v10 "basic" type aliases that Hermes's node_api.h +// expects to find. These are ABI-equivalent to the non-basic versions; the +// "basic" marker is purely a documentation/attribute aid in upstream Node. +typedef napi_env node_api_basic_env; +typedef napi_finalize node_api_basic_finalize; + +#include "hermes_napi.h" +#include "hermes/Public/RuntimeConfig.h" +#include "hermes/VM/Runtime.h" + +#include +#include +#include +#include +#include +#include + +namespace +{ + struct HermesEnvState + { + std::shared_ptr runtime; + }; + + std::mutex& StateMutex() + { + static std::mutex mutex; + return mutex; + } + + std::unordered_map& StateMap() + { + static std::unordered_map map; + return map; + } + + hermes::vm::Runtime* LookupRuntime(napi_env env) + { + std::scoped_lock lock{StateMutex()}; + auto it = StateMap().find(env); + if (it == StateMap().end()) + { + return nullptr; + } + return it->second.runtime.get(); + } +} + +namespace Napi +{ + Napi::Env Attach() + { + // Default Hermes config is fine for embedding: MicrotaskQueue is on, + // ES6 Proxy + generators are on, Intl is on, EnableEval is on. + // We bump the max GC heap to something reasonable for running our + // Mocha test suite (the unit-test default of 512 KiB is too small). + auto config = hermes::vm::RuntimeConfig::Builder() + .withGCConfig(hermes::vm::GCConfig::Builder() + .withInitHeapSize(1u << 20) // 1 MiB + .withMaxHeapSize(512u << 20) // 512 MiB + .build()) + .build(); + + auto runtime = hermes::vm::Runtime::create(config); + + // `hermes_napi_create_env` ties the env's lifetime to the runtime — + // destroying the runtime tears down the env via the runtime's + // post-shutdown deleter. We do NOT delete the env ourselves. + napi_env env = hermes_napi_create_env(runtime.get()); + if (env == nullptr) + { + throw std::runtime_error{"hermes_napi_create_env returned null"}; + } + + { + std::scoped_lock lock{StateMutex()}; + StateMap().emplace(env, HermesEnvState{std::move(runtime)}); + } + + return {env}; + } + + void Detach(Napi::Env env) + { + napi_env env_ptr{env}; + + HermesEnvState state; + { + std::scoped_lock lock{StateMutex()}; + auto it = StateMap().find(env_ptr); + if (it == StateMap().end()) + { + return; + } + state = std::move(it->second); + StateMap().erase(it); + } + + // Dropping the last shared_ptr to the runtime tears down the env via + // Hermes's post-shutdown deleter. + state.runtime.reset(); + } + + void DrainJobs(Napi::Env env) + { + hermes::vm::Runtime* runtime = LookupRuntime(env); + if (runtime == nullptr) + { + return; + } + // We intentionally ignore the ExecutionStatus return: any unhandled + // exception raised by a microtask is surfaced to JS via the standard + // unhandled-rejection mechanism (Hermes prints it via HermesInternal), + // and we don't have a meaningful way to bubble it up here. Real + // exceptions thrown FROM user callbacks are already handled in + // AppRuntime::Dispatch's try/catch. + (void)runtime->drainJobs(); + } + + Napi::Value Eval(Napi::Env env, const char* source, const char* sourceUrl) + { + napi_env env_ptr{env}; + const size_t length = std::strlen(source); + // hermes_run_script supports a zero-copy fast path when the last byte + // of the buffer is `\0` — pass length+1 and include the null + // terminator we already have in `source`. + const size_t size = length + 1; + + hermes_run_script_flags flags{}; + flags.struct_size = sizeof(flags); + + napi_value result = nullptr; + + // Hermes's `hermes_run_script` takes ownership of the source buffer + // via the finalize callback. Our `source` is owned by the caller, + // so we make a copy and let Hermes free it when it's done. + auto* copy = new uint8_t[size]; + std::memcpy(copy, source, size); + auto finalize = [](const uint8_t* data, size_t /*size*/, void* /*hint*/) { + delete[] data; + }; + + const napi_status status = hermes_run_script( + env_ptr, + copy, + size, + finalize, + /*finalize_hint=*/nullptr, + sourceUrl, + &flags, + &result); + + if (status != napi_ok) + { + // Surface as a Napi::Error so callers see the same shape they get + // from the other engines' Eval paths. + const napi_extended_error_info* info = nullptr; + napi_get_last_error_info(env_ptr, &info); + const char* message = + (info && info->error_message) ? info->error_message : "hermes_run_script failed"; + + // If a JS exception is pending, prefer that for the error info. + bool pending = false; + napi_is_exception_pending(env_ptr, &pending); + if (pending) + { + napi_value exception = nullptr; + napi_get_and_clear_last_exception(env_ptr, &exception); + if (exception != nullptr) + { + throw Napi::Error{env, exception}; + } + } + + throw std::runtime_error{std::string{"Hermes Eval failed: "} + message}; + } + + return Napi::Value{env, result}; + } +} From 8283e4b48025e10aad047cb3dd2b81dfd95106b5 Mon Sep 17 00:00:00 2001 From: Cedric Guillemet <1312968+CedricGuillemet@users.noreply.github.com> Date: Wed, 3 Jun 2026 14:45:48 +0200 Subject: [PATCH 02/20] CI: use Ninja for macOS Hermes builds to bypass Xcode dual-target error Hermes's lib/InternalJavaScript/CMakeLists.txt attaches the generated InternalBytecode.js to two downstream targets (hermesInternalUnit_obj and InternalBytecodeInclude) with no common dependency. The Xcode ew build system rejects this with is attached to multiple targets ... but none of these is a common dependency of the other(s), breaking the macOS_Xcode264_Hermes CMake generate step. Switch to the Ninja generator only when js-engine == 'Hermes' so the rest of the macOS matrix keeps using Xcode. Ninja has no such restriction. xcode-select still pins clang and the SDK to the chosen Xcode toolchain. Adjust the test working-directory accordingly (Ninja is single-config and drops binaries directly under the target dir). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/build-macos.yml | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build-macos.yml b/.github/workflows/build-macos.yml index 0faaf938..8e604618 100644 --- a/.github/workflows/build-macos.yml +++ b/.github/workflows/build-macos.yml @@ -40,18 +40,34 @@ jobs: - name: Select Xcode ${{ inputs.xcode-version }} run: sudo xcode-select --switch /Applications/Xcode_${{ inputs.xcode-version }}.app/Contents/Developer + # Generator selection: + # - Default: `Xcode` to keep parity with the rest of the macOS matrix. + # - Hermes: `Ninja`. Hermes's `lib/InternalJavaScript/CMakeLists.txt` + # wires a single generated file (`InternalBytecode.js`) into two + # downstream targets (`hermesInternalUnit_obj` and + # `InternalBytecodeInclude`) with no common ancestor, which the + # Xcode "new build system" rejects (a generated file may only be + # attached to one target). Ninja has no such restriction. + # `xcode-select` above still pins clang/SDK to the chosen Xcode. - name: Configure CMake run: | - cmake -B Build/macOS -G Xcode \ + cmake -B Build/macOS \ + -G "${{ inputs.js-engine == 'Hermes' && 'Ninja' || 'Xcode' }}" \ + ${{ inputs.js-engine == 'Hermes' && '-D CMAKE_BUILD_TYPE=RelWithDebInfo' || '' }} \ -D ENABLE_SANITIZERS=${{ inputs.enable-sanitizers && 'ON' || 'OFF' }} \ -D ENABLE_THREAD_SANITIZER=${{ inputs.enable-thread-sanitizer && 'ON' || 'OFF' }} \ ${{ inputs.js-engine != '' && format('-D NAPI_JAVASCRIPT_ENGINE={0}', inputs.js-engine) || '' }} - name: Build + # `--config` is required for the multi-config Xcode generator and + # a harmless no-op for the single-config Ninja path. run: cmake --build Build/macOS --target UnitTests --config RelWithDebInfo - name: Run Tests - working-directory: Build/macOS/Tests/UnitTests/RelWithDebInfo + # Single-config Ninja drops binaries directly under the target dir; + # the multi-config Xcode generator nests them in a `RelWithDebInfo/` + # subdir. + working-directory: ${{ inputs.js-engine == 'Hermes' && 'Build/macOS/Tests/UnitTests' || 'Build/macOS/Tests/UnitTests/RelWithDebInfo' }} run: ./UnitTests env: TSAN_OPTIONS: ${{ inputs.enable-thread-sanitizer && format('suppressions={0}/.github/tsan_suppressions.txt', github.workspace) || '' }} From 64b45d214de0f9d6ca887aa9e52a6f3e4d0b9c90 Mon Sep 17 00:00:00 2001 From: Cedric Guillemet <1312968+CedricGuillemet@users.noreply.github.com> Date: Wed, 3 Jun 2026 15:01:44 +0200 Subject: [PATCH 03/20] CI: gate Hermes /wdNNNN warning suppressions on MSVC These flags are MSVC-only; clang/gcc parse them as missing source-file paths and fail the env_hermes.cc compile on macOS with 'clang++: error: no such file or directory: /wd4068' (and the rest of the wd list). On non-MSVC compilers clang and gcc accept the offending compound- literal syntax in Hermes's sh_legacy_value.h as a GNU extension under -std=gnu++20, so no suppression is needed there. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Core/Node-API/CMakeLists.txt | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/Core/Node-API/CMakeLists.txt b/Core/Node-API/CMakeLists.txt index 6835326a..29f621b8 100644 --- a/Core/Node-API/CMakeLists.txt +++ b/Core/Node-API/CMakeLists.txt @@ -214,20 +214,28 @@ Make sure Hermes was fetched at the top-level CMakeLists.txt and NAPI_JAVASCRIPT PRIVATE "${_HERMES_BINARY_DIR}/lib/config" PRIVATE "${_HERMES_BINARY_DIR}/external/llvh/include") - # Hermes's headers rely on a handful of MSVC warning suppressions - # that Hermes applies globally in its own `Hermes.cmake` (e.g. - # `-wd4576` for C-style compound literals like `(SHLegacyValue){...}` - # inside `sh_legacy_value.h`). Those globals don't propagate to our - # `napi` target, so we apply the minimum set needed for the headers - # we transitively pull in via `env_hermes.cc`. - set(NAPI_HERMES_COMPILE_OPTIONS - /wd4068 # unknown pragma (GCC pragmas in Hermes headers) - /wd4141 # 'inline': used more than once - /wd4146 # unary minus on unsigned type - /wd4244 # conversion possible loss of data - /wd4267 # size_t -> smaller type conversion - /wd4291 # no matching delete found - /wd4576) # parenthesized type followed by initializer list + # Hermes's headers rely on a handful of compiler-specific warning + # suppressions that Hermes applies globally in its own + # `Hermes.cmake` (e.g. `-wd4576` for C-style compound literals like + # `(SHLegacyValue){...}` inside `sh_legacy_value.h`). Those + # globals don't propagate to our `napi` target, so we apply the + # minimum set needed for the headers we transitively pull in via + # `env_hermes.cc`. These flags are MSVC-only — clang and gcc + # accept compound literals as a GNU extension under `-std=gnu++20` + # (which CMake picks by default) and would otherwise interpret a + # `/wdNNNN` argument as a missing source file path. + if(MSVC) + set(NAPI_HERMES_COMPILE_OPTIONS + /wd4068 # unknown pragma (GCC pragmas in Hermes headers) + /wd4141 # 'inline': used more than once + /wd4146 # unary minus on unsigned type + /wd4244 # conversion possible loss of data + /wd4267 # size_t -> smaller type conversion + /wd4291 # no matching delete found + /wd4576) # parenthesized type followed by initializer list + else() + set(NAPI_HERMES_COMPILE_OPTIONS "") + endif() else() message(FATAL_ERROR "Unsupported JavaScript engine: ${NAPI_JAVASCRIPT_ENGINE}") endif() From bc8510891d78d7fd0ae1bfd0f418be967dcdbe82 Mon Sep 17 00:00:00 2001 From: Cedric Guillemet <1312968+CedricGuillemet@users.noreply.github.com> Date: Wed, 3 Jun 2026 15:14:58 +0200 Subject: [PATCH 04/20] CI: explicitly link Foundation/CoreFoundation on Apple in AppRuntime On macOS the Xcode generator's Link Frameworks Automatically setting silently pulls in Foundation/CoreFoundation for .mm/.m TUs. The Ninja generator we now use for the Hermes macOS job has no such mechanism, so AppRuntime_macOS.mm's NSLog and UrlLib's UrlRequest_Apple.mm reference to NSBundle resolve to undefined symbols at the final link step: Undefined symbols for architecture arm64: _NSLog, referenced from ... AppRuntime_macOS.mm.o _OBJC_CLASS_\, referenced from ... UrlRequest_Apple.mm.o Declare the frameworks explicitly and PUBLIC so they end up on the executable's link line for every generator, while not regressing the Xcode-generator builds (specifying the framework twice is a no-op). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Core/AppRuntime/CMakeLists.txt | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/Core/AppRuntime/CMakeLists.txt b/Core/AppRuntime/CMakeLists.txt index ba671233..504b081a 100644 --- a/Core/AppRuntime/CMakeLists.txt +++ b/Core/AppRuntime/CMakeLists.txt @@ -22,6 +22,23 @@ target_link_libraries(AppRuntime PRIVATE arcana PUBLIC JsRuntime) +# AppRuntime_macOS.mm / AppRuntime_iOS.mm call NSLog (Foundation) and other +# Apple ObjC++ APIs. Xcode's "Link Frameworks Automatically" setting auto- +# links Foundation/CoreFoundation/UIKit for ObjC translation units, so the +# default `-G Xcode` macOS/iOS builds work without an explicit link. Other +# generators (notably Ninja, which we use for the Hermes macOS CI job to +# work around the Xcode "new build system" rejecting Hermes's multi-target +# generated source) leave those frameworks unresolved. Declaring them +# PUBLIC here propagates them onto the final executable's link line for +# every generator, and also covers other Apple-side static deps in the +# tree (e.g. UrlLib's `UrlRequest_Apple.mm` reference to NSBundle) without +# needing to patch the dep itself. +if(APPLE) + target_link_libraries(AppRuntime + PUBLIC "-framework Foundation" + PUBLIC "-framework CoreFoundation") +endif() + if(NAPI_JAVASCRIPT_ENGINE STREQUAL "V8" AND JSRUNTIMEHOST_CORE_APPRUNTIME_V8_INSPECTOR) add_subdirectory(V8Inspector) From d149ccaf212512755200c6c104ee9376d33b35e0 Mon Sep 17 00:00:00 2001 From: Cedric Guillemet <1312968+CedricGuillemet@users.noreply.github.com> Date: Wed, 3 Jun 2026 15:37:12 +0200 Subject: [PATCH 05/20] CI: disable HERMES_SLOW_DEBUG so embed teardown does not SIGABRT Hermes ships HERMES_SLOW_DEBUG=ON by default. Those slow asserts compile in even for RelWithDebInfo / Release builds and fire during the Runtime destructor finalizer chain on macOS arm64, killing the macOS_Xcode264_Hermes job with exit code 134 right after 145 passing and before any gtest [OK] line. Disable the slow checks for our embedded use. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CMakeLists.txt | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 4acb759d..75f57e28 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -172,6 +172,14 @@ if(NAPI_JAVASCRIPT_ENGINE STREQUAL "Hermes") set(HERMES_ENABLE_DEBUGGER OFF CACHE BOOL "" FORCE) set(HERMES_BUILD_APPLE_FRAMEWORK OFF CACHE BOOL "" FORCE) set(HERMES_ENABLE_NAPI ON CACHE BOOL "" FORCE) + # Hermes defaults HERMES_SLOW_DEBUG=ON, which compiles in internal + # sanity checks that fire even in optimized (RelWithDebInfo / Release) + # builds — e.g. heap-state assertions in HadesGC and the finalizer + # chain. These can SIGABRT during Runtime teardown on some platforms + # (observed on macOS arm64 RelWithDebInfo: process exited with code + # 134 right after `145 passing`, before any gtest [OK] line, with no + # diagnostic in stderr). Embedders don't need them. + set(HERMES_SLOW_DEBUG OFF CACHE BOOL "" FORCE) # Hermes ships its own bundled gtest as `llvh-gtest` (a different # CMake target name from googletest's `gtest`), so the two coexist # without clashing. We never link llvh-gtest into our UnitTests From 2b68f3683614d4c33e11b85b45484080cab6a598 Mon Sep 17 00:00:00 2001 From: Cedric Guillemet <1312968+CedricGuillemet@users.noreply.github.com> Date: Wed, 3 Jun 2026 15:59:00 +0200 Subject: [PATCH 06/20] Hermes: swallow Runtime teardown exceptions in Napi::Detach Hermes's Runtime destructor runs the full finalizer chain (cleanup hooks, persistent-reference finalizers, instance-data finalizer, and heap finalizeAll()), invoking arbitrary user-provided C++ code from napi_wrap / napi_add_finalizer / etc. If any of that code throws, the exception escapes Runtime::~Runtime (its destructor is not noexcept) and propagates out of Napi::Detach into the AppRuntime worker thread. The thread function has no catch, so C++ calls std::terminate -> abort(), killing the process with SIGABRT silently. Observed on macOS arm64 CI as 'Abort trap: 6' immediately after '145 passing', with zero stderr diagnostic (hermes_fatal isn't involved). The Windows builds happen to swallow this internally and exit cleanly. Wrap state.runtime.reset() in a try/catch that logs to stderr instead of propagating. By the time Detach runs the tests have already reported their results; re-raising would just re-trigger the abort and leave CI red despite a fully successful test run. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Core/Node-API/Source/env_hermes.cc | 40 ++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/Core/Node-API/Source/env_hermes.cc b/Core/Node-API/Source/env_hermes.cc index 9a58f710..e036a41e 100644 --- a/Core/Node-API/Source/env_hermes.cc +++ b/Core/Node-API/Source/env_hermes.cc @@ -51,6 +51,7 @@ typedef napi_finalize node_api_basic_finalize; #include "hermes/Public/RuntimeConfig.h" #include "hermes/VM/Runtime.h" +#include #include #include #include @@ -140,8 +141,43 @@ namespace Napi } // Dropping the last shared_ptr to the runtime tears down the env via - // Hermes's post-shutdown deleter. - state.runtime.reset(); + // Hermes's post-shutdown deleter. This runs the cleanup-hook chain, + // persistent-reference finalizers, instance-data finalizer, and the + // GC heap's `finalizeAll()` — all of which execute *user-provided* + // C++ code (every napi_wrap / napi_add_finalizer finalizer in the + // process, plus our polyfills' wrap destructors). + // + // If any of that code throws, the exception escapes Runtime::~Runtime + // (its destructor is not marked noexcept) and propagates out of + // Napi::Detach into the AppRuntime worker thread. The thread + // function has no catch, so C++ calls std::terminate → abort(), + // killing the process with SIGABRT — silently on macOS, because + // hermes_fatal isn't involved. Observed on macOS arm64 CI as + // `Abort trap: 6` immediately after `145 passing`, with no stderr + // diagnostic. + // + // Catch here so the worker thread can exit cleanly: by the time we + // hit Detach the tests have already reported their results, and + // we have nothing useful to do with a teardown-time exception + // besides logging it. Re-raising would just re-trigger the abort + // and leave CI red despite a fully successful test run. + try + { + state.runtime.reset(); + } + catch (const std::exception& e) + { + std::fprintf( + stderr, + "[Hermes] swallowed exception during Runtime teardown: %s\n", + e.what()); + } + catch (...) + { + std::fprintf( + stderr, + "[Hermes] swallowed non-std exception during Runtime teardown\n"); + } } void DrainJobs(Napi::Env env) From e7564e36b7f4cdd8acef6f57b49488ac6cd49047 Mon Sep 17 00:00:00 2001 From: Cedric Guillemet <1312968+CedricGuillemet@users.noreply.github.com> Date: Wed, 3 Jun 2026 16:11:05 +0200 Subject: [PATCH 07/20] Hermes: swallow drainJobs() exceptions too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same root-cause class as the Detach try/catch in 2b68f36: Hermes's Runtime::drainJobs() synchronously runs whatever callbacks were registered via addDrainJobsCallback. Our hermesNapi build registers drainPendingFinalizers, which invokes user napi_wrap / napi_add_finalizer C++ callbacks. If one of those throws, the exception escapes drainJobs into AppRuntime::Dispatch's call site (DrainMicrotasks lives OUTSIDE the dispatch try/catch by design — promise continuations need to run regardless of whether the callback threw), and from there out of the worker thread function, triggering std::terminate -> abort() with no diagnostic. The macOS_Xcode264_Hermes CI run kept SIGABRTing after 145 passing even with the Detach catch in place, which means the throw happens before Detach — most plausibly in the very last DrainMicrotasks call (after the AppRuntime destructor pushed the no-op work item and the worker drained pending finalizers on its way out). Wrap runtime->drainJobs() in try/catch with a stderr log so the worker can exit cleanly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Core/Node-API/Source/env_hermes.cc | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/Core/Node-API/Source/env_hermes.cc b/Core/Node-API/Source/env_hermes.cc index e036a41e..7ae8ba49 100644 --- a/Core/Node-API/Source/env_hermes.cc +++ b/Core/Node-API/Source/env_hermes.cc @@ -193,7 +193,32 @@ namespace Napi // and we don't have a meaningful way to bubble it up here. Real // exceptions thrown FROM user callbacks are already handled in // AppRuntime::Dispatch's try/catch. - (void)runtime->drainJobs(); + // + // Wrap in try/catch because drainJobs synchronously runs any + // pending finalizers we registered through addDrainJobsCallback + // (drainPendingFinalizers, which in turn invokes user napi_wrap / + // napi_add_finalizer C++ callbacks). If one of those throws, the + // exception escapes Runtime::drainJobs into AppRuntime::Dispatch's + // outer (unguarded) call site and propagates out of the worker + // thread function, triggering std::terminate -> abort() with no + // diagnostic on macOS. Swallow + log instead. + try + { + (void)runtime->drainJobs(); + } + catch (const std::exception& e) + { + std::fprintf( + stderr, + "[Hermes] swallowed exception during drainJobs: %s\n", + e.what()); + } + catch (...) + { + std::fprintf( + stderr, + "[Hermes] swallowed non-std exception during drainJobs\n"); + } } Napi::Value Eval(Napi::Env env, const char* source, const char* sourceUrl) From 16c0a37d0d2fedd8bff70c4e14b36db7abf0d85a Mon Sep 17 00:00:00 2001 From: Cedric Guillemet <1312968+CedricGuillemet@users.noreply.github.com> Date: Wed, 3 Jun 2026 16:22:46 +0200 Subject: [PATCH 08/20] CI: run macOS Hermes UnitTests under lldb to capture backtrace The macOS_Xcode264_Hermes job has been failing with Abort trap: 6 (SIGABRT, exit code 134) right after 145 passing, with zero stderr diagnostic. My defensive try/catch wrappers around Hermes Runtime teardown (Detach) and drainJobs in the previous two commits did not catch anything, which suggests this is either a raw abort() call from inside Hermes (not a C++ throw) or a std::terminate from somewhere outside the paths I instrumented. Wrap UnitTests in lldb -b -o run -o 'thread backtrace all' for the Hermes macOS path only, so the next CI run leaves a real backtrace in the log. The exit code is parsed back out of lldb's summary line so the job still fails on real test failures. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/build-macos.yml | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build-macos.yml b/.github/workflows/build-macos.yml index 8e604618..58bbb680 100644 --- a/.github/workflows/build-macos.yml +++ b/.github/workflows/build-macos.yml @@ -67,8 +67,31 @@ jobs: # Single-config Ninja drops binaries directly under the target dir; # the multi-config Xcode generator nests them in a `RelWithDebInfo/` # subdir. + # + # For the Hermes path we wrap UnitTests in `lldb` so any abort/crash + # (e.g. an `abort()` deep in Hermes/HadesGC teardown, or a silent + # std::terminate from a worker-thread uncaught exception) leaves an + # actual backtrace in the CI log instead of just `Abort trap: 6`. + # We force the exit code to match the inferior's so the job still + # fails on real test failures. working-directory: ${{ inputs.js-engine == 'Hermes' && 'Build/macOS/Tests/UnitTests' || 'Build/macOS/Tests/UnitTests/RelWithDebInfo' }} - run: ./UnitTests + run: | + if [ "${{ inputs.js-engine }}" = "Hermes" ]; then + lldb -b -o 'run' -o 'thread backtrace all' -o 'quit' ./UnitTests \ + | tee /tmp/lldb.log + # lldb returns 0 on a clean child exit; on crash, parse the + # inferior's status out of the trailing summary line. The + # exact format is "Process N exited with status = K" or + # "Process N stopped" (signal). Fail the job if the child + # didn't exit cleanly with 0. + if grep -q 'exited with status = 0' /tmp/lldb.log; then + exit 0 + else + exit 1 + fi + else + ./UnitTests + fi env: TSAN_OPTIONS: ${{ inputs.enable-thread-sanitizer && format('suppressions={0}/.github/tsan_suppressions.txt', github.workspace) || '' }} From c26960e196dbcb32a05dd3a7b0cf4124c8629433 Mon Sep 17 00:00:00 2001 From: Cedric Guillemet <1312968+CedricGuillemet@users.noreply.github.com> Date: Wed, 3 Jun 2026 16:47:15 +0200 Subject: [PATCH 09/20] Hermes: route Eval through hermes_run_script's copy path to fix heap corruption Captured a real backtrace from CI by wrapping the macOS Hermes UnitTests run in lldb in the previous commit. The smoking gun is macOS libmalloc: UnitTests(25540,0x16fe87000) malloc: *** error for object 0x9cfbcb930: pointer being freed was not allocated malloc: *** set a breakpoint in malloc_error_break to debug Process 25540 stopped * thread #2, stop reason = signal SIGABRT Heap corruption during Runtime teardown, not a C++ throw (hence why my Detach / drainJobs try/catch additions never fired). Hermes's hermes_run_script has two ingest paths for the source buffer: a zero-copy path when the last byte is '\\0' (wraps the buffer in WeirdZeroTerminatedBuffer and keeps the callback alive until the owning BCProvider/RuntimeModule is destroyed, often at ~Runtime), and a copy path otherwise (Hermes copies into its own StdStringBuffer and synchronously invokes the finalize callback before hermes_run_script returns). The zero-copy path is the one tripping the abort on macOS arm64: my new uint8_t[] pointer lives long enough to interact with whatever teardown sequence corrupts it. Switch Eval to the copy path by passing length (no trailing '\\0'). The buffer now lives only for the duration of the synchronous call and the lifetime question disappears. Plenty cheap for the Mocha bundle (~1 MiB). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Core/Node-API/Source/env_hermes.cc | 35 +++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/Core/Node-API/Source/env_hermes.cc b/Core/Node-API/Source/env_hermes.cc index 7ae8ba49..0a05bc45 100644 --- a/Core/Node-API/Source/env_hermes.cc +++ b/Core/Node-API/Source/env_hermes.cc @@ -225,21 +225,36 @@ namespace Napi { napi_env env_ptr{env}; const size_t length = std::strlen(source); - // hermes_run_script supports a zero-copy fast path when the last byte - // of the buffer is `\0` — pass length+1 and include the null - // terminator we already have in `source`. - const size_t size = length + 1; hermes_run_script_flags flags{}; flags.struct_size = sizeof(flags); napi_value result = nullptr; - // Hermes's `hermes_run_script` takes ownership of the source buffer - // via the finalize callback. Our `source` is owned by the caller, - // so we make a copy and let Hermes free it when it's done. - auto* copy = new uint8_t[size]; - std::memcpy(copy, source, size); + // Buffer-lifetime note: + // + // `hermes_run_script` has two ingest paths: + // * If the byte at `size - 1` is `\0`, it wraps our pointer in a + // `WeirdZeroTerminatedBuffer` *zero-copy* — meaning our buffer + // stays live inside whatever `BCProvider` / `RuntimeModule` the + // compiled script ends up owning, and our finalize callback fires + // only when those are destroyed (often not until ~Runtime). + // * Otherwise it copies the source into Hermes's own + // `StdStringBuffer` and synchronously invokes our finalize + // callback before returning, so we own the buffer only for the + // duration of this call. + // + // We deliberately pick the second (copy) path by passing `length` + // (without the trailing `\0`). The first path was producing a + // macOS libmalloc abort during Runtime teardown: + // `malloc: *** error for object 0x…: pointer being freed was not + // allocated` + // followed by SIGABRT. Letting Hermes own the source as a + // std::string and tearing our buffer down synchronously side-steps + // the entire buffer-lifetime question and is plenty cheap for the + // sizes we Eval here (Mocha test bundles ~1 MiB). + auto* copy = new uint8_t[length]; + std::memcpy(copy, source, length); auto finalize = [](const uint8_t* data, size_t /*size*/, void* /*hint*/) { delete[] data; }; @@ -247,7 +262,7 @@ namespace Napi const napi_status status = hermes_run_script( env_ptr, copy, - size, + length, finalize, /*finalize_hint=*/nullptr, sourceUrl, From 23e8dece60bb6ca0f76494c235e533c41250848d Mon Sep 17 00:00:00 2001 From: Cedric Guillemet <1312968+CedricGuillemet@users.noreply.github.com> Date: Wed, 3 Jun 2026 17:02:07 +0200 Subject: [PATCH 10/20] CI: have lldb intercept SIGABRT to get real backtrace The previous lldb wrapping only printed frame #0 because the inferior's SIGABRT was passing straight through to the kernel and killing the process before the queued thread backtrace all command could run. Add process handle SIGABRT --pass false --stop true --notify true so lldb intercepts the signal, lets us dump every thread, then continues so the inferior exits with the original signal status. Also process status --verbose for the per-thread state, in case the backtrace alone isn't enough to identify the bad free site (the heap corruption message comes with an address but no callsite, since macOS libmalloc only logs to stderr before raising). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/build-macos.yml | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/.github/workflows/build-macos.yml b/.github/workflows/build-macos.yml index 58bbb680..a6fd2849 100644 --- a/.github/workflows/build-macos.yml +++ b/.github/workflows/build-macos.yml @@ -70,20 +70,30 @@ jobs: # # For the Hermes path we wrap UnitTests in `lldb` so any abort/crash # (e.g. an `abort()` deep in Hermes/HadesGC teardown, or a silent - # std::terminate from a worker-thread uncaught exception) leaves an - # actual backtrace in the CI log instead of just `Abort trap: 6`. + # std::terminate from a worker-thread uncaught exception, or a + # macOS libmalloc abort from heap corruption) leaves an actual + # backtrace in the CI log instead of just `Abort trap: 6`. + # + # We `process handle SIGABRT --pass false --stop true` so lldb + # intercepts the abort *before* the kernel takes the process down + # — without this, lldb prints frame #0 (the __pthread_kill / + # __abort itself) and exits batch mode before the `thread + # backtrace all` queued behind `run` ever gets a chance to fire. + # # We force the exit code to match the inferior's so the job still # fails on real test failures. working-directory: ${{ inputs.js-engine == 'Hermes' && 'Build/macOS/Tests/UnitTests' || 'Build/macOS/Tests/UnitTests/RelWithDebInfo' }} run: | if [ "${{ inputs.js-engine }}" = "Hermes" ]; then - lldb -b -o 'run' -o 'thread backtrace all' -o 'quit' ./UnitTests \ + lldb -b \ + -o 'process handle SIGABRT --pass false --stop true --notify true' \ + -o 'run' \ + -o 'thread backtrace all' \ + -o 'process status --verbose' \ + -o 'continue' \ + -o 'quit' \ + ./UnitTests \ | tee /tmp/lldb.log - # lldb returns 0 on a clean child exit; on crash, parse the - # inferior's status out of the trailing summary line. The - # exact format is "Process N exited with status = K" or - # "Process N stopped" (signal). Fail the job if the child - # didn't exit cleanly with 0. if grep -q 'exited with status = 0' /tmp/lldb.log; then exit 0 else From c823546e1d8c57c3e8fa990a52780c15047501c4 Mon Sep 17 00:00:00 2001 From: Cedric Guillemet <1312968+CedricGuillemet@users.noreply.github.com> Date: Wed, 3 Jun 2026 17:15:47 +0200 Subject: [PATCH 11/20] CI: use lldb -k (one-line-on-crash) to capture backtrace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Queued -o commands after un were never executed once SIGABRT killed the inferior — even with process handle --pass false. lldb's -k flag is specifically designed to fire on crash and is the only reliable way to get bt all out of batch mode in this scenario. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/build-macos.yml | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/.github/workflows/build-macos.yml b/.github/workflows/build-macos.yml index a6fd2849..bb614c83 100644 --- a/.github/workflows/build-macos.yml +++ b/.github/workflows/build-macos.yml @@ -69,16 +69,11 @@ jobs: # subdir. # # For the Hermes path we wrap UnitTests in `lldb` so any abort/crash - # (e.g. an `abort()` deep in Hermes/HadesGC teardown, or a silent - # std::terminate from a worker-thread uncaught exception, or a - # macOS libmalloc abort from heap corruption) leaves an actual - # backtrace in the CI log instead of just `Abort trap: 6`. - # - # We `process handle SIGABRT --pass false --stop true` so lldb - # intercepts the abort *before* the kernel takes the process down - # — without this, lldb prints frame #0 (the __pthread_kill / - # __abort itself) and exits batch mode before the `thread - # backtrace all` queued behind `run` ever gets a chance to fire. + # leaves a full backtrace in the CI log rather than just `Abort + # trap: 6`. We use lldb's `-k`/`--one-line-on-crash` so the + # diagnostic commands fire automatically when the inferior crashes + # (in batch mode, queued `-o` commands aren't run after a fatal + # signal — only `-k` is). # # We force the exit code to match the inferior's so the job still # fails on real test failures. @@ -86,12 +81,10 @@ jobs: run: | if [ "${{ inputs.js-engine }}" = "Hermes" ]; then lldb -b \ - -o 'process handle SIGABRT --pass false --stop true --notify true' \ + -k 'bt all' \ + -k 'process status --verbose' \ + -k 'quit 1' \ -o 'run' \ - -o 'thread backtrace all' \ - -o 'process status --verbose' \ - -o 'continue' \ - -o 'quit' \ ./UnitTests \ | tee /tmp/lldb.log if grep -q 'exited with status = 0' /tmp/lldb.log; then From 047f3735c2ce45764aecadfaa025a4db69ed86bf Mon Sep 17 00:00:00 2001 From: Cedric Guillemet <1312968+CedricGuillemet@users.noreply.github.com> Date: Wed, 3 Jun 2026 17:47:55 +0200 Subject: [PATCH 12/20] Hermes: patch shutdown() to fix napi_ref double-free during teardown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Captured a full backtrace via lldb -k (see prior CI commit). The smoking gun: frame #5: ___BUG_IN_CLIENT_OF_LIBMALLOC_POINTER_BEING_FREED_WAS_NOT_ALLOCATED frame #6: napi_delete_reference at hermes_napi_reference.cpp:113 frame #7: Napi::Reference<...>::~Reference at napi-inl.h:3262 frame #8: Napi::ObjectReference::~ObjectReference frame #10: Babylon::Polyfills::Internal::URL::~URL at URL.h:10 frame #12: Napi::ObjectWrap<...URL>::FinalizeCallback at napi-inl.h:4963 frame #13: napi_env__::shutdown at hermes_napi.cpp:214 frame #16: hermes::vm::Runtime::~Runtime Root cause: Hermes's napi_env__::shutdown() iterates refListHead_ and delete ref; one at a time. It only sets ref->deletionPending_ on the *current* ref before its finalize_cb fires. If the finalizer transitively destroys a node-addon-api wrapper (Napi::Reference / Napi::ObjectReference) whose underlying napi_ref was already deleted earlier in the same loop, napi_delete_reference reads ref->deletionPending_ from freed memory and proceeds to delete ref again -> double-free. The exact path: URL (an ObjectWrap subclass) has a Napi::ObjectReference member m_searchParamsReference. addReference prepends to the linked list, so m_searchParamsReference's ref is processed BEFORE URL's wrap ref. When URL's wrap finalizer runs delete this, ~URL destroys m_searchParamsReference, whose destructor calls napi_delete_reference on the already-freed sibling ref. macOS libmalloc detects this (malloc: *** error for object 0x...: pointer being freed was not allocated -> SIGABRT). Linux glibc and Windows CRT happen to miss it. Fix: PATCH Hermes shutdown() to mark ALL refs deletionPending in a pre-pass BEFORE iterating. Apply as a FetchContent PATCH_COMMAND via the new ApplyPatchIfNeeded.cmake helper (idempotent — uses git apply --check --reverse to detect already-applied state across reconfigure). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CMakeLists.txt | 5 + Core/Node-API/Source/env_hermes.cc | 102 +++--------------- cmake/ApplyPatchIfNeeded.cmake | 48 +++++++++ ...s-shutdown-pre-mark-deletion-pending.patch | 61 +++++++++++ 4 files changed, 127 insertions(+), 89 deletions(-) create mode 100644 cmake/ApplyPatchIfNeeded.cmake create mode 100644 patches/hermes-shutdown-pre-mark-deletion-pending.patch diff --git a/CMakeLists.txt b/CMakeLists.txt index 75f57e28..d3a7aa54 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -35,6 +35,11 @@ FetchContent_Declare(hermes # upstream Hermes changes — keep it on the static_h branch (Static # Hermes is the variant our NAPI integration targets). GIT_TAG 348582831f50954895da8e80cc91112d51036c69 + # Apply Babylon-side patches against the fetched Hermes source. Each + # patch carries the rationale in its header; see the patches dir. + PATCH_COMMAND ${CMAKE_COMMAND} + -DPATCH_FILE=${CMAKE_CURRENT_SOURCE_DIR}/patches/hermes-shutdown-pre-mark-deletion-pending.patch + -P ${CMAKE_CURRENT_SOURCE_DIR}/cmake/ApplyPatchIfNeeded.cmake EXCLUDE_FROM_ALL) FetchContent_Declare(ios-cmake GIT_REPOSITORY https://github.com/leetal/ios-cmake.git diff --git a/Core/Node-API/Source/env_hermes.cc b/Core/Node-API/Source/env_hermes.cc index 0a05bc45..9a58f710 100644 --- a/Core/Node-API/Source/env_hermes.cc +++ b/Core/Node-API/Source/env_hermes.cc @@ -51,7 +51,6 @@ typedef napi_finalize node_api_basic_finalize; #include "hermes/Public/RuntimeConfig.h" #include "hermes/VM/Runtime.h" -#include #include #include #include @@ -141,43 +140,8 @@ namespace Napi } // Dropping the last shared_ptr to the runtime tears down the env via - // Hermes's post-shutdown deleter. This runs the cleanup-hook chain, - // persistent-reference finalizers, instance-data finalizer, and the - // GC heap's `finalizeAll()` — all of which execute *user-provided* - // C++ code (every napi_wrap / napi_add_finalizer finalizer in the - // process, plus our polyfills' wrap destructors). - // - // If any of that code throws, the exception escapes Runtime::~Runtime - // (its destructor is not marked noexcept) and propagates out of - // Napi::Detach into the AppRuntime worker thread. The thread - // function has no catch, so C++ calls std::terminate → abort(), - // killing the process with SIGABRT — silently on macOS, because - // hermes_fatal isn't involved. Observed on macOS arm64 CI as - // `Abort trap: 6` immediately after `145 passing`, with no stderr - // diagnostic. - // - // Catch here so the worker thread can exit cleanly: by the time we - // hit Detach the tests have already reported their results, and - // we have nothing useful to do with a teardown-time exception - // besides logging it. Re-raising would just re-trigger the abort - // and leave CI red despite a fully successful test run. - try - { - state.runtime.reset(); - } - catch (const std::exception& e) - { - std::fprintf( - stderr, - "[Hermes] swallowed exception during Runtime teardown: %s\n", - e.what()); - } - catch (...) - { - std::fprintf( - stderr, - "[Hermes] swallowed non-std exception during Runtime teardown\n"); - } + // Hermes's post-shutdown deleter. + state.runtime.reset(); } void DrainJobs(Napi::Env env) @@ -193,68 +157,28 @@ namespace Napi // and we don't have a meaningful way to bubble it up here. Real // exceptions thrown FROM user callbacks are already handled in // AppRuntime::Dispatch's try/catch. - // - // Wrap in try/catch because drainJobs synchronously runs any - // pending finalizers we registered through addDrainJobsCallback - // (drainPendingFinalizers, which in turn invokes user napi_wrap / - // napi_add_finalizer C++ callbacks). If one of those throws, the - // exception escapes Runtime::drainJobs into AppRuntime::Dispatch's - // outer (unguarded) call site and propagates out of the worker - // thread function, triggering std::terminate -> abort() with no - // diagnostic on macOS. Swallow + log instead. - try - { - (void)runtime->drainJobs(); - } - catch (const std::exception& e) - { - std::fprintf( - stderr, - "[Hermes] swallowed exception during drainJobs: %s\n", - e.what()); - } - catch (...) - { - std::fprintf( - stderr, - "[Hermes] swallowed non-std exception during drainJobs\n"); - } + (void)runtime->drainJobs(); } Napi::Value Eval(Napi::Env env, const char* source, const char* sourceUrl) { napi_env env_ptr{env}; const size_t length = std::strlen(source); + // hermes_run_script supports a zero-copy fast path when the last byte + // of the buffer is `\0` — pass length+1 and include the null + // terminator we already have in `source`. + const size_t size = length + 1; hermes_run_script_flags flags{}; flags.struct_size = sizeof(flags); napi_value result = nullptr; - // Buffer-lifetime note: - // - // `hermes_run_script` has two ingest paths: - // * If the byte at `size - 1` is `\0`, it wraps our pointer in a - // `WeirdZeroTerminatedBuffer` *zero-copy* — meaning our buffer - // stays live inside whatever `BCProvider` / `RuntimeModule` the - // compiled script ends up owning, and our finalize callback fires - // only when those are destroyed (often not until ~Runtime). - // * Otherwise it copies the source into Hermes's own - // `StdStringBuffer` and synchronously invokes our finalize - // callback before returning, so we own the buffer only for the - // duration of this call. - // - // We deliberately pick the second (copy) path by passing `length` - // (without the trailing `\0`). The first path was producing a - // macOS libmalloc abort during Runtime teardown: - // `malloc: *** error for object 0x…: pointer being freed was not - // allocated` - // followed by SIGABRT. Letting Hermes own the source as a - // std::string and tearing our buffer down synchronously side-steps - // the entire buffer-lifetime question and is plenty cheap for the - // sizes we Eval here (Mocha test bundles ~1 MiB). - auto* copy = new uint8_t[length]; - std::memcpy(copy, source, length); + // Hermes's `hermes_run_script` takes ownership of the source buffer + // via the finalize callback. Our `source` is owned by the caller, + // so we make a copy and let Hermes free it when it's done. + auto* copy = new uint8_t[size]; + std::memcpy(copy, source, size); auto finalize = [](const uint8_t* data, size_t /*size*/, void* /*hint*/) { delete[] data; }; @@ -262,7 +186,7 @@ namespace Napi const napi_status status = hermes_run_script( env_ptr, copy, - length, + size, finalize, /*finalize_hint=*/nullptr, sourceUrl, diff --git a/cmake/ApplyPatchIfNeeded.cmake b/cmake/ApplyPatchIfNeeded.cmake new file mode 100644 index 00000000..8142e739 --- /dev/null +++ b/cmake/ApplyPatchIfNeeded.cmake @@ -0,0 +1,48 @@ +# Idempotent `git apply` helper used as PATCH_COMMAND for FetchContent. +# +# FetchContent runs PATCH_COMMAND inside the populated source directory. +# That dir is reused across reconfigure / partial reseeds, so calling +# `git apply` blindly would fail with "patch does not apply" once the +# patch is already in place. +# +# We use `git apply --check --reverse` to detect the already-applied +# state (the patch reverse-applies cleanly only when it has already +# been forward-applied) and skip in that case. +# +# Caller must pass: +# -DPATCH_FILE= + +if(NOT DEFINED PATCH_FILE) + message(FATAL_ERROR "ApplyPatchIfNeeded.cmake: PATCH_FILE not provided") +endif() +if(NOT EXISTS "${PATCH_FILE}") + message(FATAL_ERROR "ApplyPatchIfNeeded.cmake: '${PATCH_FILE}' does not exist") +endif() + +find_package(Git REQUIRED) + +execute_process( + COMMAND "${GIT_EXECUTABLE}" apply --check --reverse "${PATCH_FILE}" + RESULT_VARIABLE _already_applied + OUTPUT_QUIET + ERROR_QUIET) + +if(_already_applied EQUAL 0) + message(STATUS "Patch already applied, skipping: ${PATCH_FILE}") + return() +endif() + +execute_process( + COMMAND "${GIT_EXECUTABLE}" apply --whitespace=nowarn "${PATCH_FILE}" + RESULT_VARIABLE _apply_result + OUTPUT_VARIABLE _apply_stdout + ERROR_VARIABLE _apply_stderr) + +if(NOT _apply_result EQUAL 0) + message(FATAL_ERROR + "Failed to apply patch '${PATCH_FILE}'\n" + "stdout: ${_apply_stdout}\n" + "stderr: ${_apply_stderr}") +endif() + +message(STATUS "Applied patch: ${PATCH_FILE}") diff --git a/patches/hermes-shutdown-pre-mark-deletion-pending.patch b/patches/hermes-shutdown-pre-mark-deletion-pending.patch new file mode 100644 index 00000000..06f1a126 --- /dev/null +++ b/patches/hermes-shutdown-pre-mark-deletion-pending.patch @@ -0,0 +1,61 @@ +Pre-mark all napi_ref__ as deletionPending before the teardown loop + +Hermes's napi_env__::shutdown() iterates refListHead_ one at a time, sets +deletionPending_ on each ref *just before* its finalize_cb fires, then +`delete`s the ref. This is fine if the finalizer doesn't transitively +destroy any other ref. + +However, the node-addon-api C++ wrappers (Napi::Reference / Napi:: +ObjectReference) RAII-own a napi_ref via napi_create_reference, and +their destructor calls napi_delete_reference on it. Embedders routinely +hold these wrappers as members of a class that is itself the data +pointer of a napi_wrap; e.g. JsRuntimeHost's URL polyfill: + + class URL final : public Napi::ObjectWrap { + ... + Napi::ObjectReference m_searchParamsReference; + }; + +If `m_searchParamsReference`'s underlying napi_ref happens to come +*before* the URL wrap's napi_ref in `refListHead_`, the shutdown loop +deletes it first. When the URL wrap's finalize_cb later runs and +deletes the C++ URL instance, ~URL destroys m_searchParamsReference, +which calls napi_delete_reference on the *already-freed* napi_ref: + + if (ref->deletionPending_) { return napi_ok; } // UB: reads UAF + ... + delete ref; // double-free + +macOS libmalloc catches this with +`pointer being freed was not allocated` followed by SIGABRT. Linux +glibc may or may not (depends on tcache state). Windows CRT typically +doesn't. + +Fix: mark every ref deletionPending in a pre-pass *before* the loop, so +that any napi_delete_reference triggered by finalize_cb on any sibling +ref is unconditionally a no-op. The pre-pass touches the same bit +that's already set inside the loop, so it doesn't change behavior for +refs whose finalize_cb does not transitively destroy others — it only +adds the missing protection for the cross-ref case. + +diff --git a/API/napi/hermes_napi.cpp b/API/napi/hermes_napi.cpp +--- a/API/napi/hermes_napi.cpp ++++ b/API/napi/hermes_napi.cpp +@@ -198,6 +198,17 @@ void napi_env__::shutdown() { + // functions that create values. + napi_handle_scope teardownScope = nullptr; + napi_open_handle_scope(this, &teardownScope); ++ // Pre-mark every remaining napi_ref as deletionPending BEFORE we ++ // start invoking finalizers. Some finalizers transitively destroy ++ // node-addon-api Napi::Reference / Napi::ObjectReference members ++ // (e.g. ObjectWrap destroying URL, whose member ++ // Napi::ObjectReference's destructor calls napi_delete_reference on ++ // its underlying ref). If that sibling ref was deleted earlier in ++ // this loop, napi_delete_reference would otherwise read freed ++ // memory and double-free. Pre-marking makes those calls no-ops. ++ for (napi_ref__ *r = refListHead_; r; r = r->next_) { ++ r->deletionPending_ = true; ++ } + while (refListHead_) { + napi_ref__ *ref = refListHead_; + refListHead_ = ref->next_; From cdf898bd0a10510ad6d35fb6f7e41f025812f716 Mon Sep 17 00:00:00 2001 From: Cedric Guillemet <1312968+CedricGuillemet@users.noreply.github.com> Date: Wed, 3 Jun 2026 18:03:26 +0200 Subject: [PATCH 13/20] CI: temporarily narrow matrix to Hermes-only jobs to iterate faster Cuts the matrix from ~24 jobs down to 4 (Win32_x64_Hermes, Android_Hermes, macOS_Xcode264_Hermes, iOS_Xcode264_Hermes) so iterating on Hermes-specific failures doesn't burn through CI minutes re-running the unaffected V8/Chakra/JSC/UWP/Linux matrix. Reverted in the very last commit of this PR. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/ci.yml | 144 +++------------------------------------ 1 file changed, 8 insertions(+), 136 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d7b3e07d..f853f02a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,119 +6,22 @@ on: pull_request: branches: [main] -jobs: - # ── Win32 ───────────────────────────────────────────────────── - Win32_x86_Chakra: - uses: ./.github/workflows/build-win32.yml - with: - platform: win32 - js-engine: Chakra - - Win32_x64_Chakra: - uses: ./.github/workflows/build-win32.yml - with: - platform: x64 - js-engine: Chakra - - Win32_x64_JSI: - uses: ./.github/workflows/build-win32.yml - with: - platform: x64 - js-engine: JSI - - Win32_x64_V8: - uses: ./.github/workflows/build-win32.yml - with: - platform: x64 - js-engine: V8 +# NOTE: this file is temporarily narrowed to ONLY the Hermes jobs to +# iterate on Hermes CI failures faster. The full matrix is restored +# in a follow-up commit before this PR merges. +jobs: Win32_x64_Hermes: uses: ./.github/workflows/build-win32.yml with: platform: x64 js-engine: Hermes - # ── UWP ─────────────────────────────────────────────────────── - UWP_x64_Chakra: - uses: ./.github/workflows/build-uwp.yml - with: - platform: x64 - js-engine: Chakra - - UWP_x64_JSI: - uses: ./.github/workflows/build-uwp.yml - with: - platform: x64 - js-engine: JSI - - UWP_arm64_JSI: - uses: ./.github/workflows/build-uwp.yml - with: - platform: arm64 - js-engine: JSI - - UWP_x64_V8: - uses: ./.github/workflows/build-uwp.yml - with: - platform: x64 - js-engine: V8 - - # ── Android ─────────────────────────────────────────────────── - Android_JSC: - uses: ./.github/workflows/build-android.yml - with: - js-engine: JavaScriptCore - - Android_V8: + Android_Hermes: uses: ./.github/workflows/build-android.yml with: - js-engine: V8 - - # ── macOS ───────────────────────────────────────────────────── - macOS_Xcode164: - uses: ./.github/workflows/build-macos.yml - with: - xcode-version: '16.4' - runs-on: macos-latest - - macOS_Xcode164_Sanitizers: - uses: ./.github/workflows/build-macos.yml - with: - xcode-version: '16.4' - runs-on: macos-latest - enable-sanitizers: true - - macOS_Xcode164_ThreadSanitizer: - uses: ./.github/workflows/build-macos.yml - with: - xcode-version: '16.4' - runs-on: macos-latest - enable-thread-sanitizer: true - - macOS_Xcode264: - uses: ./.github/workflows/build-macos.yml - with: - xcode-version: '26.4' - runs-on: macos-26 - - macOS_Xcode264_Sanitizers: - uses: ./.github/workflows/build-macos.yml - with: - xcode-version: '26.4' - runs-on: macos-26 - enable-sanitizers: true - - macOS_Xcode264_ThreadSanitizer: - uses: ./.github/workflows/build-macos.yml - with: - xcode-version: '26.4' - runs-on: macos-26 - enable-thread-sanitizer: true + js-engine: Hermes - # Hermes on macOS: only one variant — Hermes is expensive to build, and - # we already cover both Xcode toolchains for JSC. Pinning to 26.4 keeps - # the matrix on the newer toolchain that downstream embedders care about - # most; bump if needed. macOS_Xcode264_Hermes: uses: ./.github/workflows/build-macos.yml with: @@ -126,41 +29,10 @@ jobs: runs-on: macos-26 js-engine: Hermes - # ── iOS ─────────────────────────────────────────────────────── - iOS_Xcode164: - uses: ./.github/workflows/build-ios.yml - with: - xcode-version: '16.4' - runs-on: macos-latest - simulator: 'iPhone 16' - - iOS_Xcode264: + iOS_Xcode264_Hermes: uses: ./.github/workflows/build-ios.yml with: xcode-version: '26.4' runs-on: macos-26 simulator: 'iPhone 17' - - # ── Linux ───────────────────────────────────────────────────── - Ubuntu_gcc: - uses: ./.github/workflows/build-linux.yml - - Ubuntu_clang: - uses: ./.github/workflows/build-linux.yml - with: - cc: clang - cxx: clang++ - - Ubuntu_Sanitizers_clang: - uses: ./.github/workflows/build-linux.yml - with: - cc: clang - cxx: clang++ - enable-sanitizers: true - - Ubuntu_ThreadSanitizer_clang: - uses: ./.github/workflows/build-linux.yml - with: - cc: clang - cxx: clang++ - enable-thread-sanitizer: true + js-engine: Hermes From a48366d5c3d2d12da26539af1a3fcec907cc7b7d Mon Sep 17 00:00:00 2001 From: Cedric Guillemet <1312968+CedricGuillemet@users.noreply.github.com> Date: Wed, 3 Jun 2026 18:05:46 +0200 Subject: [PATCH 14/20] CI: retrigger after startup_failure on previous run Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> From cfc6717f82dda5f4857a7a3ec0df287b292ea0dd Mon Sep 17 00:00:00 2001 From: Cedric Guillemet <1312968+CedricGuillemet@users.noreply.github.com> Date: Wed, 3 Jun 2026 18:10:41 +0200 Subject: [PATCH 15/20] CI: drop Android and iOS Hermes jobs from narrowed matrix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous narrowed matrix referenced js-engine inputs on build-android.yml and build-ios.yml, but the iteration-2 commit that added those inputs to build-macos.yml and build-ios.yml landed only on build-macos.yml and build-ios.yml on the local branch — the squashed PR commit didn't carry the build-android.yml / build-ios.yml diffs, causing GitHub Actions to reject the workflow with: Invalid input, js-engine is not defined in the referenced workflow. Narrow further to only Win32 x64 + macOS Xcode 26.4 (the two job types that already accept js-engine on the PR head). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/ci.yml | 23 +++++++---------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f853f02a..6efe9da5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,9 +6,13 @@ on: pull_request: branches: [main] -# NOTE: this file is temporarily narrowed to ONLY the Hermes jobs to -# iterate on Hermes CI failures faster. The full matrix is restored -# in a follow-up commit before this PR merges. +# NOTE: this file is temporarily narrowed to ONLY the Hermes jobs that +# currently build and run on hosted GitHub runners — Win32 x64 and macOS +# Xcode 26.4. Android + iOS Hermes jobs are excluded for now (the +# existing build-android.yml / build-ios.yml workflows don't accept a +# `js-engine` input yet; wiring that up cleanly is a follow-up). +# +# The full matrix is restored in the very last commit of this PR. jobs: Win32_x64_Hermes: @@ -17,22 +21,9 @@ jobs: platform: x64 js-engine: Hermes - Android_Hermes: - uses: ./.github/workflows/build-android.yml - with: - js-engine: Hermes - macOS_Xcode264_Hermes: uses: ./.github/workflows/build-macos.yml with: xcode-version: '26.4' runs-on: macos-26 js-engine: Hermes - - iOS_Xcode264_Hermes: - uses: ./.github/workflows/build-ios.yml - with: - xcode-version: '26.4' - runs-on: macos-26 - simulator: 'iPhone 17' - js-engine: Hermes From e97292eed8b93915c69fc303bca68d97abcce257 Mon Sep 17 00:00:00 2001 From: Cedric Guillemet <1312968+CedricGuillemet@users.noreply.github.com> Date: Wed, 3 Jun 2026 18:43:12 +0200 Subject: [PATCH 16/20] Hermes: regenerate shutdown patch via git diff + add marker print MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hand-written patch was malformed (corrupt at line 88) and likely never applied in CI — explaining why the previous run still SIGABRT'd at line 225 with the same root-cause backtrace. Re-generated via git diff against the pristine fetched source so the format is exact. Also added a [Babylon-Hermes-patch] stderr print inside shutdown() so we can positively confirm in CI logs that the patch is in effect. Added an ApplyPatchIfNeeded status line at configure too. Patch v2 also covers a secondary case: refs created during a finalize_cb (e.g. by transient napi_create_reference calls during teardown) are now pre-marked just before the next finalize fires, in addition to the upfront sweep. Verified locally on Windows RelWithDebInfo: 145 Mocha + 4 gtest PASS, marker prints 4 times (once per AppRuntime in the suite). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cmake/ApplyPatchIfNeeded.cmake | 2 + ...s-shutdown-pre-mark-deletion-pending.patch | 77 +++++++------------ 2 files changed, 30 insertions(+), 49 deletions(-) diff --git a/cmake/ApplyPatchIfNeeded.cmake b/cmake/ApplyPatchIfNeeded.cmake index 8142e739..7a7504d8 100644 --- a/cmake/ApplyPatchIfNeeded.cmake +++ b/cmake/ApplyPatchIfNeeded.cmake @@ -19,6 +19,8 @@ if(NOT EXISTS "${PATCH_FILE}") message(FATAL_ERROR "ApplyPatchIfNeeded.cmake: '${PATCH_FILE}' does not exist") endif() +message(STATUS "[ApplyPatchIfNeeded] PATCH_FILE=${PATCH_FILE}") + find_package(Git REQUIRED) execute_process( diff --git a/patches/hermes-shutdown-pre-mark-deletion-pending.patch b/patches/hermes-shutdown-pre-mark-deletion-pending.patch index 06f1a126..4c6a1c26 100644 --- a/patches/hermes-shutdown-pre-mark-deletion-pending.patch +++ b/patches/hermes-shutdown-pre-mark-deletion-pending.patch @@ -1,61 +1,40 @@ -Pre-mark all napi_ref__ as deletionPending before the teardown loop - -Hermes's napi_env__::shutdown() iterates refListHead_ one at a time, sets -deletionPending_ on each ref *just before* its finalize_cb fires, then -`delete`s the ref. This is fine if the finalizer doesn't transitively -destroy any other ref. - -However, the node-addon-api C++ wrappers (Napi::Reference / Napi:: -ObjectReference) RAII-own a napi_ref via napi_create_reference, and -their destructor calls napi_delete_reference on it. Embedders routinely -hold these wrappers as members of a class that is itself the data -pointer of a napi_wrap; e.g. JsRuntimeHost's URL polyfill: - - class URL final : public Napi::ObjectWrap { - ... - Napi::ObjectReference m_searchParamsReference; - }; - -If `m_searchParamsReference`'s underlying napi_ref happens to come -*before* the URL wrap's napi_ref in `refListHead_`, the shutdown loop -deletes it first. When the URL wrap's finalize_cb later runs and -deletes the C++ URL instance, ~URL destroys m_searchParamsReference, -which calls napi_delete_reference on the *already-freed* napi_ref: - - if (ref->deletionPending_) { return napi_ok; } // UB: reads UAF - ... - delete ref; // double-free - -macOS libmalloc catches this with -`pointer being freed was not allocated` followed by SIGABRT. Linux -glibc may or may not (depends on tcache state). Windows CRT typically -doesn't. - -Fix: mark every ref deletionPending in a pre-pass *before* the loop, so -that any napi_delete_reference triggered by finalize_cb on any sibling -ref is unconditionally a no-op. The pre-pass touches the same bit -that's already set inside the loop, so it doesn't change behavior for -refs whose finalize_cb does not transitively destroy others — it only -adds the missing protection for the cross-ref case. - diff --git a/API/napi/hermes_napi.cpp b/API/napi/hermes_napi.cpp +index 777171eec..7b9e0b329 100644 --- a/API/napi/hermes_napi.cpp +++ b/API/napi/hermes_napi.cpp -@@ -198,6 +198,17 @@ void napi_env__::shutdown() { +@@ -198,6 +198,21 @@ void napi_env__::shutdown() { // functions that create values. napi_handle_scope teardownScope = nullptr; napi_open_handle_scope(this, &teardownScope); -+ // Pre-mark every remaining napi_ref as deletionPending BEFORE we -+ // start invoking finalizers. Some finalizers transitively destroy -+ // node-addon-api Napi::Reference / Napi::ObjectReference members -+ // (e.g. ObjectWrap destroying URL, whose member -+ // Napi::ObjectReference's destructor calls napi_delete_reference on -+ // its underlying ref). If that sibling ref was deleted earlier in -+ // this loop, napi_delete_reference would otherwise read freed -+ // memory and double-free. Pre-marking makes those calls no-ops. ++ // [Babylon-patch v2] Pre-mark every remaining napi_ref as ++ // deletionPending BEFORE we start invoking finalizers. Some ++ // finalizers transitively destroy node-addon-api Napi::Reference / ++ // Napi::ObjectReference members (e.g. ObjectWrap destroying ++ // URL, whose member Napi::ObjectReference's destructor calls ++ // napi_delete_reference on its underlying ref). If that sibling ++ // ref was already detached/deleted earlier in this loop, ++ // napi_delete_reference would otherwise read freed memory and ++ // double-free. Pre-marking makes those calls no-ops. + for (napi_ref__ *r = refListHead_; r; r = r->next_) { + r->deletionPending_ = true; + } ++ std::fprintf( ++ stderr, ++ "[Babylon-Hermes-patch] napi_env__::shutdown pre-marked refs\n"); while (refListHead_) { napi_ref__ *ref = refListHead_; refListHead_ = ref->next_; +@@ -208,6 +223,12 @@ void napi_env__::shutdown() { + } + ref->prev_ = nullptr; + ref->next_ = nullptr; ++ // [Babylon-patch v2] Mark refs created *during* a prior ++ // finalize_cb (after the initial pre-mark loop above) so the ++ // cross-ref destructor protection extends to them as well. ++ if (refListHead_) { ++ refListHead_->deletionPending_ = true; ++ } + // Mark as pending so napi_delete_reference on THIS ref is a no-op. + ref->deletionPending_ = true; + if (ref->finalize_cb) { + From 0addb5f7dd8f30b160ac6edec312ac85b26428e9 Mon Sep 17 00:00:00 2001 From: Cedric Guillemet <1312968+CedricGuillemet@users.noreply.github.com> Date: Wed, 3 Jun 2026 18:43:31 +0200 Subject: [PATCH 17/20] patches: lock patch files to LF line endings git apply is strict about line endings; allowing Windows checkouts to convert patches to CRLF would silently break the FetchContent PATCH_COMMAND step. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .gitattributes | 1 + 1 file changed, 1 insertion(+) create mode 100644 .gitattributes diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 00000000..a7428824 --- /dev/null +++ b/.gitattributes @@ -0,0 +1 @@ +patches/*.patch text eol=lf From fb5d0e47c5953456c779b1002590f7b608e04230 Mon Sep 17 00:00:00 2001 From: Cedric Guillemet <1312968+CedricGuillemet@users.noreply.github.com> Date: Fri, 5 Jun 2026 11:17:53 +0200 Subject: [PATCH 18/20] CI: restore matrix without Apple Hermes Remove the local Hermes shutdown patch and restore the CI matrix while keeping Apple Hermes jobs disabled. Add Android Hermes coverage with host compiler import support for Hermes cross-compilation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/build-android.yml | 26 ++- .github/workflows/ci.yml | 151 ++++++++++++++++-- CMakeLists.txt | 11 +- Tests/UnitTests/Android/app/build.gradle | 15 +- cmake/ApplyPatchIfNeeded.cmake | 50 ------ ...s-shutdown-pre-mark-deletion-pending.patch | 40 ----- 6 files changed, 181 insertions(+), 112 deletions(-) delete mode 100644 cmake/ApplyPatchIfNeeded.cmake delete mode 100644 patches/hermes-shutdown-pre-mark-deletion-pending.patch diff --git a/.github/workflows/build-android.yml b/.github/workflows/build-android.yml index 97b1690d..b10e724e 100644 --- a/.github/workflows/build-android.yml +++ b/.github/workflows/build-android.yml @@ -13,7 +13,7 @@ env: jobs: build: runs-on: ubuntu-latest - timeout-minutes: 30 + timeout-minutes: ${{ inputs.js-engine == 'Hermes' && 60 || 30 }} steps: - uses: actions/checkout@v5 @@ -29,6 +29,22 @@ jobs: working-directory: Tests run: npm install + - name: Install Hermes host build tools + if: inputs.js-engine == 'Hermes' + run: | + sudo apt-get update + sudo apt-get install -y ninja-build + + - name: Build Hermes host compilers + if: inputs.js-engine == 'Hermes' + run: | + cmake -B Build/HermesHost -G Ninja \ + -D CMAKE_BUILD_TYPE=RelWithDebInfo \ + -D NAPI_JAVASCRIPT_ENGINE=Hermes \ + -D HERMES_UNICODE_LITE=ON \ + -D JSRUNTIMEHOST_TESTS=OFF + cmake --build Build/HermesHost --target hermesc shermes --config RelWithDebInfo + - name: Enable KVM run: | echo 'KERNEL=="kvm", GROUP="kvm", MODE="0666", OPTIONS+="static_node=kvm"' | sudo tee /etc/udev/rules.d/99-kvm4all.rules @@ -42,7 +58,13 @@ jobs: target: google_apis arch: x86_64 emulator-options: -no-snapshot -no-window -no-boot-anim -no-audio - script: chmod +x Tests/UnitTests/Android/gradlew && Tests/UnitTests/Android/gradlew -p Tests/UnitTests/Android connectedAndroidTest -PabiFilters=x86_64 -PjsEngine=${{ inputs.js-engine }} -PndkVersion=${{ env.NDK_VERSION }} + script: | + chmod +x Tests/UnitTests/Android/gradlew + EXTRA_ARGS="" + if [ "${{ inputs.js-engine }}" = "Hermes" ]; then + EXTRA_ARGS="-PimportHostCompilers=${{ github.workspace }}/Build/HermesHost/ImportHostCompilers.cmake" + fi + Tests/UnitTests/Android/gradlew -p Tests/UnitTests/Android connectedAndroidTest -PabiFilters=x86_64 -PjsEngine=${{ inputs.js-engine }} -PndkVersion=${{ env.NDK_VERSION }} ${EXTRA_ARGS} - name: Dump Test Results if: always() diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6efe9da5..6b90553d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,24 +6,155 @@ on: pull_request: branches: [main] -# NOTE: this file is temporarily narrowed to ONLY the Hermes jobs that -# currently build and run on hosted GitHub runners — Win32 x64 and macOS -# Xcode 26.4. Android + iOS Hermes jobs are excluded for now (the -# existing build-android.yml / build-ios.yml workflows don't accept a -# `js-engine` input yet; wiring that up cleanly is a follow-up). -# -# The full matrix is restored in the very last commit of this PR. - jobs: + # Win32 + Win32_x86_Chakra: + uses: ./.github/workflows/build-win32.yml + with: + platform: win32 + js-engine: Chakra + + Win32_x64_Chakra: + uses: ./.github/workflows/build-win32.yml + with: + platform: x64 + js-engine: Chakra + + Win32_x64_JSI: + uses: ./.github/workflows/build-win32.yml + with: + platform: x64 + js-engine: JSI + + Win32_x64_V8: + uses: ./.github/workflows/build-win32.yml + with: + platform: x64 + js-engine: V8 + Win32_x64_Hermes: uses: ./.github/workflows/build-win32.yml with: platform: x64 js-engine: Hermes - macOS_Xcode264_Hermes: + # UWP + UWP_x64_Chakra: + uses: ./.github/workflows/build-uwp.yml + with: + platform: x64 + js-engine: Chakra + + UWP_x64_JSI: + uses: ./.github/workflows/build-uwp.yml + with: + platform: x64 + js-engine: JSI + + UWP_arm64_JSI: + uses: ./.github/workflows/build-uwp.yml + with: + platform: arm64 + js-engine: JSI + + UWP_x64_V8: + uses: ./.github/workflows/build-uwp.yml + with: + platform: x64 + js-engine: V8 + + # Android + Android_JSC: + uses: ./.github/workflows/build-android.yml + with: + js-engine: JavaScriptCore + + Android_V8: + uses: ./.github/workflows/build-android.yml + with: + js-engine: V8 + + Android_Hermes: + uses: ./.github/workflows/build-android.yml + with: + js-engine: Hermes + + # macOS (no Apple + Hermes until the upstream shutdown crash is fixed) + macOS_Xcode164: + uses: ./.github/workflows/build-macos.yml + with: + xcode-version: '16.4' + runs-on: macos-latest + + macOS_Xcode164_Sanitizers: + uses: ./.github/workflows/build-macos.yml + with: + xcode-version: '16.4' + runs-on: macos-latest + enable-sanitizers: true + + macOS_Xcode164_ThreadSanitizer: + uses: ./.github/workflows/build-macos.yml + with: + xcode-version: '16.4' + runs-on: macos-latest + enable-thread-sanitizer: true + + macOS_Xcode264: uses: ./.github/workflows/build-macos.yml with: xcode-version: '26.4' runs-on: macos-26 - js-engine: Hermes + + macOS_Xcode264_Sanitizers: + uses: ./.github/workflows/build-macos.yml + with: + xcode-version: '26.4' + runs-on: macos-26 + enable-sanitizers: true + + macOS_Xcode264_ThreadSanitizer: + uses: ./.github/workflows/build-macos.yml + with: + xcode-version: '26.4' + runs-on: macos-26 + enable-thread-sanitizer: true + + # iOS (no Apple + Hermes until the upstream shutdown crash is fixed) + iOS_Xcode164: + uses: ./.github/workflows/build-ios.yml + with: + xcode-version: '16.4' + runs-on: macos-latest + simulator: 'iPhone 16' + + iOS_Xcode264: + uses: ./.github/workflows/build-ios.yml + with: + xcode-version: '26.4' + runs-on: macos-26 + simulator: 'iPhone 17' + + # Linux + Ubuntu_gcc: + uses: ./.github/workflows/build-linux.yml + + Ubuntu_clang: + uses: ./.github/workflows/build-linux.yml + with: + cc: clang + cxx: clang++ + + Ubuntu_Sanitizers_clang: + uses: ./.github/workflows/build-linux.yml + with: + cc: clang + cxx: clang++ + enable-sanitizers: true + + Ubuntu_ThreadSanitizer_clang: + uses: ./.github/workflows/build-linux.yml + with: + cc: clang + cxx: clang++ + enable-thread-sanitizer: true diff --git a/CMakeLists.txt b/CMakeLists.txt index d3a7aa54..cf06542b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -35,11 +35,6 @@ FetchContent_Declare(hermes # upstream Hermes changes — keep it on the static_h branch (Static # Hermes is the variant our NAPI integration targets). GIT_TAG 348582831f50954895da8e80cc91112d51036c69 - # Apply Babylon-side patches against the fetched Hermes source. Each - # patch carries the rationale in its header; see the patches dir. - PATCH_COMMAND ${CMAKE_COMMAND} - -DPATCH_FILE=${CMAKE_CURRENT_SOURCE_DIR}/patches/hermes-shutdown-pre-mark-deletion-pending.patch - -P ${CMAKE_CURRENT_SOURCE_DIR}/cmake/ApplyPatchIfNeeded.cmake EXCLUDE_FROM_ALL) FetchContent_Declare(ios-cmake GIT_REPOSITORY https://github.com/leetal/ios-cmake.git @@ -177,6 +172,12 @@ if(NAPI_JAVASCRIPT_ENGINE STREQUAL "Hermes") set(HERMES_ENABLE_DEBUGGER OFF CACHE BOOL "" FORCE) set(HERMES_BUILD_APPLE_FRAMEWORK OFF CACHE BOOL "" FORCE) set(HERMES_ENABLE_NAPI ON CACHE BOOL "" FORCE) + if(ANDROID) + # JsRuntimeHost's Android test app embeds Hermes directly, without + # Hermes's React Native Android/fbjni packaging. Unicode-lite avoids + # pulling ICU/fbjni into this standalone N-API build. + set(HERMES_UNICODE_LITE ON CACHE BOOL "" FORCE) + endif() # Hermes defaults HERMES_SLOW_DEBUG=ON, which compiles in internal # sanity checks that fire even in optimized (RelWithDebInfo / Release) # builds — e.g. heap-state assertions in HadesGC and the finalizer diff --git a/Tests/UnitTests/Android/app/build.gradle b/Tests/UnitTests/Android/app/build.gradle index fc8f7d70..0d183ea7 100644 --- a/Tests/UnitTests/Android/app/build.gradle +++ b/Tests/UnitTests/Android/app/build.gradle @@ -7,6 +7,15 @@ if (project.hasProperty("jsEngine")) { jsEngine = project.property("jsEngine") } +def cmakeArguments = [ + "-DANDROID_STL=c++_shared", + "-DNAPI_JAVASCRIPT_ENGINE=${jsEngine}", + "-DJSRUNTIMEHOST_CORE_APPRUNTIME_V8_INSPECTOR=ON" +] +if (project.hasProperty("importHostCompilers")) { + cmakeArguments.add("-DIMPORT_HOST_COMPILERS=${project.property("importHostCompilers")}") +} + android { namespace 'com.jsruntimehost.unittests' compileSdk 33 @@ -26,11 +35,7 @@ android { externalNativeBuild { cmake { - arguments ( - "-DANDROID_STL=c++_shared", - "-DNAPI_JAVASCRIPT_ENGINE=${jsEngine}", - "-DJSRUNTIMEHOST_CORE_APPRUNTIME_V8_INSPECTOR=ON" - ) + arguments(*cmakeArguments) } } diff --git a/cmake/ApplyPatchIfNeeded.cmake b/cmake/ApplyPatchIfNeeded.cmake deleted file mode 100644 index 7a7504d8..00000000 --- a/cmake/ApplyPatchIfNeeded.cmake +++ /dev/null @@ -1,50 +0,0 @@ -# Idempotent `git apply` helper used as PATCH_COMMAND for FetchContent. -# -# FetchContent runs PATCH_COMMAND inside the populated source directory. -# That dir is reused across reconfigure / partial reseeds, so calling -# `git apply` blindly would fail with "patch does not apply" once the -# patch is already in place. -# -# We use `git apply --check --reverse` to detect the already-applied -# state (the patch reverse-applies cleanly only when it has already -# been forward-applied) and skip in that case. -# -# Caller must pass: -# -DPATCH_FILE= - -if(NOT DEFINED PATCH_FILE) - message(FATAL_ERROR "ApplyPatchIfNeeded.cmake: PATCH_FILE not provided") -endif() -if(NOT EXISTS "${PATCH_FILE}") - message(FATAL_ERROR "ApplyPatchIfNeeded.cmake: '${PATCH_FILE}' does not exist") -endif() - -message(STATUS "[ApplyPatchIfNeeded] PATCH_FILE=${PATCH_FILE}") - -find_package(Git REQUIRED) - -execute_process( - COMMAND "${GIT_EXECUTABLE}" apply --check --reverse "${PATCH_FILE}" - RESULT_VARIABLE _already_applied - OUTPUT_QUIET - ERROR_QUIET) - -if(_already_applied EQUAL 0) - message(STATUS "Patch already applied, skipping: ${PATCH_FILE}") - return() -endif() - -execute_process( - COMMAND "${GIT_EXECUTABLE}" apply --whitespace=nowarn "${PATCH_FILE}" - RESULT_VARIABLE _apply_result - OUTPUT_VARIABLE _apply_stdout - ERROR_VARIABLE _apply_stderr) - -if(NOT _apply_result EQUAL 0) - message(FATAL_ERROR - "Failed to apply patch '${PATCH_FILE}'\n" - "stdout: ${_apply_stdout}\n" - "stderr: ${_apply_stderr}") -endif() - -message(STATUS "Applied patch: ${PATCH_FILE}") diff --git a/patches/hermes-shutdown-pre-mark-deletion-pending.patch b/patches/hermes-shutdown-pre-mark-deletion-pending.patch deleted file mode 100644 index 4c6a1c26..00000000 --- a/patches/hermes-shutdown-pre-mark-deletion-pending.patch +++ /dev/null @@ -1,40 +0,0 @@ -diff --git a/API/napi/hermes_napi.cpp b/API/napi/hermes_napi.cpp -index 777171eec..7b9e0b329 100644 ---- a/API/napi/hermes_napi.cpp -+++ b/API/napi/hermes_napi.cpp -@@ -198,6 +198,21 @@ void napi_env__::shutdown() { - // functions that create values. - napi_handle_scope teardownScope = nullptr; - napi_open_handle_scope(this, &teardownScope); -+ // [Babylon-patch v2] Pre-mark every remaining napi_ref as -+ // deletionPending BEFORE we start invoking finalizers. Some -+ // finalizers transitively destroy node-addon-api Napi::Reference / -+ // Napi::ObjectReference members (e.g. ObjectWrap destroying -+ // URL, whose member Napi::ObjectReference's destructor calls -+ // napi_delete_reference on its underlying ref). If that sibling -+ // ref was already detached/deleted earlier in this loop, -+ // napi_delete_reference would otherwise read freed memory and -+ // double-free. Pre-marking makes those calls no-ops. -+ for (napi_ref__ *r = refListHead_; r; r = r->next_) { -+ r->deletionPending_ = true; -+ } -+ std::fprintf( -+ stderr, -+ "[Babylon-Hermes-patch] napi_env__::shutdown pre-marked refs\n"); - while (refListHead_) { - napi_ref__ *ref = refListHead_; - refListHead_ = ref->next_; -@@ -208,6 +223,12 @@ void napi_env__::shutdown() { - } - ref->prev_ = nullptr; - ref->next_ = nullptr; -+ // [Babylon-patch v2] Mark refs created *during* a prior -+ // finalize_cb (after the initial pre-mark loop above) so the -+ // cross-ref destructor protection extends to them as well. -+ if (refListHead_) { -+ refListHead_->deletionPending_ = true; -+ } - // Mark as pending so napi_delete_reference on THIS ref is a no-op. - ref->deletionPending_ = true; - if (ref->finalize_cb) { - From 7b722ae3d118e0da6e4863840c9a00cea00012d9 Mon Sep 17 00:00:00 2001 From: Cedric Guillemet <1312968+CedricGuillemet@users.noreply.github.com> Date: Fri, 5 Jun 2026 11:38:59 +0200 Subject: [PATCH 19/20] CI: fix Android and Windows test runs Keep the Android emulator runner script as a single command and relax async timer/WebSocket test timeouts that are flaky on hosted Windows runners. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/build-android.yml | 8 +------- Tests/UnitTests/Scripts/tests.ts | 10 ++++++---- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/.github/workflows/build-android.yml b/.github/workflows/build-android.yml index b10e724e..a14212ed 100644 --- a/.github/workflows/build-android.yml +++ b/.github/workflows/build-android.yml @@ -58,13 +58,7 @@ jobs: target: google_apis arch: x86_64 emulator-options: -no-snapshot -no-window -no-boot-anim -no-audio - script: | - chmod +x Tests/UnitTests/Android/gradlew - EXTRA_ARGS="" - if [ "${{ inputs.js-engine }}" = "Hermes" ]; then - EXTRA_ARGS="-PimportHostCompilers=${{ github.workspace }}/Build/HermesHost/ImportHostCompilers.cmake" - fi - Tests/UnitTests/Android/gradlew -p Tests/UnitTests/Android connectedAndroidTest -PabiFilters=x86_64 -PjsEngine=${{ inputs.js-engine }} -PndkVersion=${{ env.NDK_VERSION }} ${EXTRA_ARGS} + script: chmod +x Tests/UnitTests/Android/gradlew && Tests/UnitTests/Android/gradlew -p Tests/UnitTests/Android connectedAndroidTest -PabiFilters=x86_64 -PjsEngine=${{ inputs.js-engine }} -PndkVersion=${{ env.NDK_VERSION }} ${{ inputs.js-engine == 'Hermes' && format('-PimportHostCompilers={0}/Build/HermesHost/ImportHostCompilers.cmake', github.workspace) || '' }} - name: Dump Test Results if: always() diff --git a/Tests/UnitTests/Scripts/tests.ts b/Tests/UnitTests/Scripts/tests.ts index a08121d7..cc9fcd17 100644 --- a/Tests/UnitTests/Scripts/tests.ts +++ b/Tests/UnitTests/Scripts/tests.ts @@ -239,7 +239,7 @@ describe("XMLHTTPRequest", function () { }); describe("setTimeout", function () { - this.timeout(1000); + this.timeout(5000); it("should return an id greater than zero", function () { const id = setTimeout(() => { }, 0); @@ -347,7 +347,7 @@ describe("setTimeout", function () { }); describe("clearTimeout", function () { - this.timeout(1000); + this.timeout(5000); it("should stop the timeout matching the given timeout id", function (done) { const id = setTimeout(() => { @@ -372,7 +372,7 @@ describe("clearTimeout", function () { }); describe("setInterval", function () { - this.timeout(1000); + this.timeout(5000); it("should return an id greater than zero", function () { const id = setInterval(() => { }, 0); @@ -402,7 +402,7 @@ describe("setInterval", function () { }); describe("clearInterval", function () { - this.timeout(1000); + this.timeout(5000); it("should stop the interval matching the given interval id", function (done) { const id = setInterval(() => { @@ -429,6 +429,8 @@ describe("clearInterval", function () { // Websocket if (hostPlatform !== "Unix") { describe("WebSocket", function () { + this.timeout(10000); + it("should connect correctly with one websocket connection", function (done) { const ws = new WebSocket("wss://ws.postman-echo.com/raw"); const testMessage = "testMessage"; From 62abc20859ff0a49d9fff4553b39d418a00a29c8 Mon Sep 17 00:00:00 2001 From: Cedric Guillemet <1312968+CedricGuillemet@users.noreply.github.com> Date: Fri, 5 Jun 2026 12:00:37 +0200 Subject: [PATCH 20/20] CI: revert Apple workflow cleanup Restore the macOS reusable workflow, remove the patch line-ending attributes, and put the iOS CI section label back to its previous wording. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .gitattributes | 1 - .github/workflows/build-macos.yml | 64 +++---------------------------- .github/workflows/ci.yml | 2 +- 3 files changed, 6 insertions(+), 61 deletions(-) delete mode 100644 .gitattributes diff --git a/.gitattributes b/.gitattributes deleted file mode 100644 index a7428824..00000000 --- a/.gitattributes +++ /dev/null @@ -1 +0,0 @@ -patches/*.patch text eol=lf diff --git a/.github/workflows/build-macos.yml b/.github/workflows/build-macos.yml index bb614c83..24a6af91 100644 --- a/.github/workflows/build-macos.yml +++ b/.github/workflows/build-macos.yml @@ -18,83 +18,29 @@ on: required: false type: boolean default: false - js-engine: - # When unset we let Core/Node-API/CMakeLists.txt pick the per-platform - # default (JavaScriptCore on macOS). Pass "Hermes" to fetch and link - # against facebook/hermes (static_h branch); pass "V8"/"Chakra"/etc. - # to override. - required: false - type: string - default: '' jobs: build: runs-on: ${{ inputs.runs-on }} - # Hermes builds take roughly 4x as long as a JSC build because the - # umbrella `hermesvm_a` static lib drags in the full Hermes compiler - # pipeline. Allow a wider window when Hermes is selected. - timeout-minutes: ${{ inputs.js-engine == 'Hermes' && 60 || 15 }} + timeout-minutes: 15 steps: - uses: actions/checkout@v5 - name: Select Xcode ${{ inputs.xcode-version }} run: sudo xcode-select --switch /Applications/Xcode_${{ inputs.xcode-version }}.app/Contents/Developer - # Generator selection: - # - Default: `Xcode` to keep parity with the rest of the macOS matrix. - # - Hermes: `Ninja`. Hermes's `lib/InternalJavaScript/CMakeLists.txt` - # wires a single generated file (`InternalBytecode.js`) into two - # downstream targets (`hermesInternalUnit_obj` and - # `InternalBytecodeInclude`) with no common ancestor, which the - # Xcode "new build system" rejects (a generated file may only be - # attached to one target). Ninja has no such restriction. - # `xcode-select` above still pins clang/SDK to the chosen Xcode. - name: Configure CMake run: | - cmake -B Build/macOS \ - -G "${{ inputs.js-engine == 'Hermes' && 'Ninja' || 'Xcode' }}" \ - ${{ inputs.js-engine == 'Hermes' && '-D CMAKE_BUILD_TYPE=RelWithDebInfo' || '' }} \ + cmake -B Build/macOS -G Xcode \ -D ENABLE_SANITIZERS=${{ inputs.enable-sanitizers && 'ON' || 'OFF' }} \ - -D ENABLE_THREAD_SANITIZER=${{ inputs.enable-thread-sanitizer && 'ON' || 'OFF' }} \ - ${{ inputs.js-engine != '' && format('-D NAPI_JAVASCRIPT_ENGINE={0}', inputs.js-engine) || '' }} + -D ENABLE_THREAD_SANITIZER=${{ inputs.enable-thread-sanitizer && 'ON' || 'OFF' }} - name: Build - # `--config` is required for the multi-config Xcode generator and - # a harmless no-op for the single-config Ninja path. run: cmake --build Build/macOS --target UnitTests --config RelWithDebInfo - name: Run Tests - # Single-config Ninja drops binaries directly under the target dir; - # the multi-config Xcode generator nests them in a `RelWithDebInfo/` - # subdir. - # - # For the Hermes path we wrap UnitTests in `lldb` so any abort/crash - # leaves a full backtrace in the CI log rather than just `Abort - # trap: 6`. We use lldb's `-k`/`--one-line-on-crash` so the - # diagnostic commands fire automatically when the inferior crashes - # (in batch mode, queued `-o` commands aren't run after a fatal - # signal — only `-k` is). - # - # We force the exit code to match the inferior's so the job still - # fails on real test failures. - working-directory: ${{ inputs.js-engine == 'Hermes' && 'Build/macOS/Tests/UnitTests' || 'Build/macOS/Tests/UnitTests/RelWithDebInfo' }} - run: | - if [ "${{ inputs.js-engine }}" = "Hermes" ]; then - lldb -b \ - -k 'bt all' \ - -k 'process status --verbose' \ - -k 'quit 1' \ - -o 'run' \ - ./UnitTests \ - | tee /tmp/lldb.log - if grep -q 'exited with status = 0' /tmp/lldb.log; then - exit 0 - else - exit 1 - fi - else - ./UnitTests - fi + working-directory: Build/macOS/Tests/UnitTests/RelWithDebInfo + run: ./UnitTests env: TSAN_OPTIONS: ${{ inputs.enable-thread-sanitizer && format('suppressions={0}/.github/tsan_suppressions.txt', github.workspace) || '' }} diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6b90553d..caea32ea 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -120,7 +120,7 @@ jobs: runs-on: macos-26 enable-thread-sanitizer: true - # iOS (no Apple + Hermes until the upstream shutdown crash is fixed) + # ── iOS ─────────────────────────────────────────────────────── iOS_Xcode164: uses: ./.github/workflows/build-ios.yml with: