Improve object safety

[Vladimir Morozov (vmoroz)]

Type of change

• Refactor / object-lifetime hardening
• ⚠️ Breaking API change (pre-1.0): JSValueScope is now constructed through static
  factory methods, and JSValueScopeType is internal.

Why

Node-API values (napi_value) and references (napi_ref) have strict, environment-scoped
lifetimes, but the previous design spread responsibility for those lifetimes across several
overlapping concepts — five JSValueScope types, a separate JSModuleContext, and a
"no-context" JSReference path. That made two simple invariants hard to guarantee:

• a napi_value is valid exactly while its JSValueScope is open, and
• a JSReference is owned by the one JSRuntimeContext bound to its napi_env.

This change makes those invariants structural. It builds on the recent worker-teardown crash
fixes (#487, #492) and removes the need for the separate no-context follow-up (#495) by
eliminating that path entirely.

What

• A napi_value is valid exactly while its JSValueScope is open. Value validity flows
  entirely through the owning scope, so using a value after its scope closes fails predictably
  instead of depending on scope-type-specific handling. JSValue is correspondingly simpler.

• A JSReference is owned by the JSRuntimeContext of its napi_env. Reference cleanup
  is always posted to the owning JS thread through that context, and the finalizer never
  touches JS state off-thread, so it stays crash-safe during environment teardown.

• Exactly one JSRuntimeContext per napi_env, disposed when the env is finalized. The
  context is stored in and resolved from the env's instance data (FromEnv, which returns a
  context only when its environment handle matches the requested env), and a native/managed
  host handshake disposes it deterministically when the environment's instance data is
  finalized — without calling back into JavaScript, since the environment is going away. The
  context is bound to its environment's JS thread (entering its scope from another thread throws),
  and the env's instance-data block is reclaimed once the last context on that env is finalized.

• Removed the "no-context" concept. Every scope and reference is backed by a runtime
  context, which removes a class of teardown edge cases (and makes the no-context reference
  leak targeted by Fix no-context JSReference leak and TSFN post/release race (follow-ups to #492) #495 moot).

• Simplified JSValueScope. Replaced JSModuleContext with a lightweight
  StrongBox<object?> module holder; reduced JSValueScopeType to three internal values
  (RuntimeContext, Handle, Escapable); the public surface is now static factories —
  CreateRuntimeScope / CreateHandleScope / CreateEscapableScope / CreateModuleScope —
  plus JSRuntimeContext.Create.

• Each loaded module resolves its own instance, and disposes cleanly. A module boundary
  (CreateModuleScope) starts a fresh module holder, so when one host loads several modules a
  later module no longer displaces an earlier module's instance. An IDisposable [JSModule]
  class is disposed exactly once at environment teardown.

• Host, embedding, and generator updated to match. The native and managed hosts and the
  embedding adapters create or resolve the context explicitly, and the generated module entry
  points split into an AOT path that creates the context and a dynamic path that resolves it.

• Documentation. Added docs/concepts/runtime-model.md — the environment / context /
  lifetime model this change relies on (one napi_env per loaded module, the instance-data
  finalizer vs. environment cleanup hook, the two-slot instance-data layout, the three scope
  types, and the rules for holding napi_value / napi_ref) — plus an AGENTS.md (and thin
  CLAUDE.md / .github/copilot-instructions.md) pointing contributors and agents to it.

• Tests. Rewrote the scope and reference unit tests for the new model and added coverage
  for value escaping, context-from-env resolution, the context factory, synchronization-context
  install/restore, the module holder, off-thread disposal, disposing several IDisposable
  modules that share one context, identity-based deduplication of module disposables, and
  rejecting a runtime scope entered from another thread. Added a worker-teardown stress test that
  repeatedly loads and tears down the host to exercise the per-environment init/teardown path.

• Build hygiene. Bumped Nullability.Source (2.1.0 → 2.3.0) to clear a dotnet format
  warning on the package's vendored source file.

Testing

Built and packed in Release, then ran the full test suite — managed unit tests plus the AOT,
hosted-CLR, embedding, and worker-teardown-stress cases — on all target frameworks. All green.

Release notes

Should this change be included in the release notes: yes — Object-lifetime safety: a
napi_value's lifetime is governed by its JSValueScope and a JSReference's lifetime by the
JSRuntimeContext of its napi_env; JSValueScope is now created via static factory methods
(CreateRuntimeScope / CreateHandleScope / CreateEscapableScope) and JSValueScopeType is
internal (breaking, pre-1.0).

[Copilot]

Pull request overview

Refactors Node-API object lifetimes around environment-owned runtime contexts and explicit value-scope factories.

Changes:

• Reworks scope, reference, module-holder, and runtime-context ownership.
• Updates hosts, embedding adapters, source generation, and Hermes integration.
• Expands lifetime and worker-teardown testing; updates documentation and dependencies.

Reviewed changes

Copilot reviewed 30 out of 30 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
test/TestCases/napi-dotnet/worker_teardown_stress.js	Adds repeated worker teardown coverage.
test/TestBuilder.cs	Hardens SDK selection for test builds.
test/MockJSRuntime.cs	Mocks escapable-handle behavior.
test/JSValueScopeTests.cs	Tests the new scope model.
test/JSReferenceTests.cs	Updates reference lifetime tests.
test/GCTests.cs	Uses runtime-scope factories.
src/NodeApi/Runtime/TracingJSRuntime.cs	Migrates traced callbacks to runtime scopes.
src/NodeApi/Runtime/NodeEmbeddingNodeApiScope.cs	Creates an embedding runtime context.
src/NodeApi/Runtime/NodeEmbedding.cs	Updates embedding callback scopes.
src/NodeApi/NodeApi.csproj	Grants internal access to host and tests.
src/NodeApi/JSValueScope.cs	Introduces factory-based scope construction.
src/NodeApi/JSValue.cs	Removes no-context callback paths.
src/NodeApi/JSReference.cs	Makes references context-owned.
src/NodeApi/JSPropertyDescriptor.cs	Captures module holders.
src/NodeApi/JSError.cs	Adapts error handling to new scopes.
src/NodeApi/Interop/JSThreadSafeFunction.cs	Uses context-resolving callback scopes.
src/NodeApi/Interop/JSSynchronizationContext.cs	Adds an inline host synchronization context.
src/NodeApi/Interop/JSRuntimeContext.cs	Adds environment registration and annotations.
src/NodeApi/Interop/JSModuleContext.cs	Removes the former module context.
src/NodeApi/Interop/JSModuleBuilderOfT.cs	Stores module instances in holders.
src/NodeApi/Interop/JSCallbackDescriptor.cs	Carries module holders through callbacks.
src/NodeApi/DotNetHost/NativeHost.cs	Adds managed-host teardown registration.
src/NodeApi.Generator/ModuleGenerator.cs	Generates separate AOT and hosted entry paths.
src/NodeApi.DotNetHost/ManagedHostRegistration.cs	Defines the host teardown handshake.
src/NodeApi.DotNetHost/ManagedHost.cs	Registers and disposes managed contexts.
src/NodeApi.DotNetHost/JSMarshaller.cs	Resolves module instances from scopes.
examples/hermes-engine/HermesRuntime.cs	Migrates Hermes to scope factories.
docs/features/js-value-scopes.md	Documents the new factory API.
Directory.Packages.props	Updates Nullability.Source.
bench/Benchmarks.cs	Updates benchmark scope creation.

Suppressed comments (3)

src/NodeApi/Interop/JSRuntimeContext.cs:184

• FromEnv uses the runtime from the most recently constructed context process-wide. Because Create publicly accepts a runtime per context, creating env A with runtime A and then env B with a stateful runtime B makes FromEnv(envA) call runtimeB.GetInstanceData(envA) and potentially return B's context. Store/resolve the runtime per environment, or require the caller's runtime explicitly instead of using this global.

    public static unsafe JSRuntimeContext? FromEnv(napi_env env)
    {
        JSRuntime? runtime = s_instanceDataRuntime;
        if (runtime is null)
        {
            return null;
        }

        runtime.GetInstanceData(env, out nint instanceData).ThrowIfFailed();

src/NodeApi/Interop/JSRuntimeContext.cs:221

• Lazy creation makes disposal unsafe for a context that never opened a scope. Dispose() accesses this property, so it attempts JSSynchronizationContext.Create() while no JSValueScope is current (including instance-data finalization), throws, and skips the remaining context/annotation cleanup. Disposal should only dispose an already-created _synchronizationContext, without constructing one.

    /// <summary>
    /// Gets the synchronization context that marshals callbacks and continuations to the JS thread.
    /// A default one is created on first access, which happens while a scope for this context is
    /// current, because creating it requires the current scope's runtime and environment.
    /// </summary>
    public JSSynchronizationContext SynchronizationContext
        => _synchronizationContext ??= JSSynchronizationContext.Create();

src/NodeApi/Interop/JSRuntimeContext.cs:278

• This silently overwrites an occupied slot instead of enforcing the promised one-context-per-env invariant. The embedding adapters now construct contexts repeatedly for the same lifecycle/env, so earlier contexts remain rooted while FromEnv suddenly resolves the last one; separate AOT addons are worse because the overwritten slot may contain a GCHandle owned by another CLR heap. Reuse/reject an existing registration and provide storage that cannot cross-dereference another runtime's handle.

        ((nint*)instanceData)[s_instanceDataSlot] = ContextHandle;

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Vladimir Morozov (vmoroz)]

Tracked as a separate follow-up (the JSTsfnSynchronizationContext post-then-release race, #497), intentionally out of scope for this PR. Removing the no-context path did not dissolve it: it needs a dedicated design for gating in-flight TSFN calls against release (a napi acquire/release refcount around each post is the obvious lever, but leaves a narrow post-count-zero/finalize window). Leaving this thread open to track it.

[Copilot]

Pull request overview

Copilot reviewed 30 out of 30 changed files in this pull request and generated 2 comments.

Suppressed comments (2)

src/NodeApi/Interop/JSRuntimeContext.cs:278

• This unconditionally replaces an existing context for the same runtime/environment. The five changed Node embedding callback adapters each construct a new context, so earlier contexts (including their synchronization contexts and references) remain live while the environment finalizer disposes only the last one. Reuse the context already registered for the environment, and reject duplicate registration in the factory as a safeguard.

        ((nint*)instanceData)[s_instanceDataSlot] = ContextHandle;

src/NodeApi/Interop/JSRuntimeContext.cs:904

• ContextHandle is intentionally never freed, so this dictionary otherwise keeps every disposed ManagedHost/NativeHost annotation strongly reachable forever. Repeated worker creation therefore accumulates disposed hosts and their load-context object graphs. Clear the owning annotations after all values have been disposed.

        if (_disposableAnnotations != null)
        {
            foreach (IDisposable annotation in _disposableAnnotations.Values)
            {

[Copilot]

Pull request overview

Copilot reviewed 30 out of 30 changed files in this pull request and generated 1 comment.

Suppressed comments (4)

src/NodeApi/Interop/JSRuntimeContext.cs:123

• The fixed two-slot layout is not actually “one slot per runtime.” Two NativeAOT modules loaded into the same napi_env run in separate managed runtimes, but both use ModuleContextSlot; the later module overwrites the first module’s opaque GCHandle, and the first module’s instance-data finalizer then attempts to interpret a handle owned by the other GC heap. This can leak or crash during teardown. The instance-data representation needs per-runtime ownership that does not place multiple runtimes’ handles in the same fixed slot.

    // Env instance-data layout: one GCHandle slot per runtime sharing the napi_env. Slot 0 is the
    // module context (managed host / AOT module / embedding); slot 1 is the native host context.
    // A runtime reads and writes only its own slot, so it never dereferences the other runtime's
    // GCHandle (which belongs to a separate GC heap).
    private const int ModuleContextSlot = 0;

src/NodeApi/Interop/JSRuntimeContext.cs:136

• This strong GCHandle is intentionally never freed, so every environment permanently roots its JSRuntimeContext and everything it still references. The worker stress path creates both native- and managed-host contexts per iteration, making repeated worker teardown a guaranteed process-lifetime managed-memory leak. Keep only teardown-safe finalize-hint state alive as long as necessary, and free the context handle after the environment’s dependent finalizers can no longer use it.

    // A GCHandle rooting this context, used both as its env instance-data slot value and as the
    // finalize hint for pooled GC handles. It is intentionally never freed: pooled-handle
    // finalizers dereference it during env teardown, after this context is already disposed.

src/NodeApi/Interop/JSRuntimeContext.cs:180

• FromEnv uses whichever runtime was registered most recently process-wide. After creating contexts with different JSRuntime instances, resolving the earlier environment calls GetInstanceData on the later runtime; stateful implementations (including the per-instance MockJSRuntime) therefore return the wrong context or fail. Track the runtime/context per environment instead of storing one global runtime.

    public static unsafe JSRuntimeContext? FromEnv(napi_env env)
    {
        JSRuntime? runtime = s_instanceDataRuntime;
        if (runtime is null)
        {

src/NodeApi/Interop/JSRuntimeContext.cs:877

• Replacing an owning annotation of the same type drops the previous instance without disposing it or returning it to the caller. Because this API promises that the context owns these values, the previous resource is leaked permanently. Define replacement semantics and either dispose the replaced value or reject duplicate registration.

    public void SetDisposableAnnotation<T>(T value) where T : class, IDisposable
    {
        if (value is null) throw new ArgumentNullException(nameof(value));
        (_disposableAnnotations ??= new())[typeof(T)] = value;

[Copilot]

Pull request overview

Copilot reviewed 30 out of 30 changed files in this pull request and generated 1 comment.

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

src/NodeApi.DotNetHost/ManagedHost.cs:213

• In hosted mode this context does not own the instance-data finalizer, but the native host receives the managed teardown callback only after ManagedHost construction succeeds and fills registration. If initialization throws before that point, the catch reports the JS error but leaves this context's rooting GCHandle and synchronization context registered forever because the native host has no callback with which to dispose it. Close the scope and dispose the context on the failed-initialization path.

        bool hosted = registration != null;
        JSRuntimeContext context = new(env, runtime);
        using JSValueScope scope = JSValueScope.CreateRuntimeScope(env, context);

src/NodeApi/Interop/JSModuleBuilderOfT.cs:37

• This drops the previous ownership behavior of JSModuleContext: when module implements IDisposable, it is no longer disposed at module/context teardown. Generated module classes such as test/TestCases/napi-dotnet/ModuleClass.cs:17 rely on that contract. Register disposable module instances with the runtime context (without collapsing multiple modules onto one annotation key) so teardown still invokes Dispose().

        // Write through the holder the descriptors captured, so callbacks bound before the module
        // instance existed observe it.
        JSValueScope.Current.ModuleHolder!.Value = module;
        exports.DefineProperties(Properties.ToArray());

src/NodeApi/Interop/JSRuntimeContext.cs:280

• This assignment silently replaces an existing context in the slot without disposing it or freeing its ContextHandle, violating the one-context-per-env invariant. The default embedding path already triggers this: RuntimeLoadingCallbackAdapter registers one context, then NodeEmbeddingNodeApiScope registers another for the same environment, permanently rooting the first context and its GC handles. Resolve/reuse the registered context in embedding callbacks/scopes, and reject or safely handle duplicate registration.

        ((nint*)instanceData)[s_instanceDataSlot] = ContextHandle;

[Copilot]

Pull request overview

Copilot reviewed 30 out of 30 changed files in this pull request and generated 1 comment.

Suppressed comments (6)

Previously missed (2) — in code that hasn't changed since the last review.

src/NodeApi/JSValueScope.cs:175

• With no parent and the default null env, this calls FromEnv(default). Once any context has initialized the static runtime, that invokes napi_get_instance_data with a null environment instead of rejecting the invalid factory call. Validate that an env was supplied before attempting environment lookup.

        // Inherit the parent scope's context, else recover it from the env instance data.
        context ??= _parentScope?.RuntimeContext
            ?? JSRuntimeContext.FromEnv(env)
            ?? throw new InvalidOperationException(
                "A runtime context could not be resolved for the scope.");

src/NodeApi.DotNetHost/ManagedHost.cs:213

• The context is registered and rooted before initialization enters the try, but the native host receives the registration handle only near the end of the successful path. If initialization throws earlier, the catch returns with no handshake handle, so environment teardown cannot dispose this managed context; its instance-data GCHandle, synchronization context, and any installed resolve handlers remain rooted. Dispose the failed scope and context after reporting the JS error.

        bool hosted = registration != null;
        JSRuntimeContext context = new(env, runtime);
        using JSValueScope scope = JSValueScope.CreateRuntimeScope(env, context);

src/NodeApi/JSReference.cs:365

• This concurrent-disposal assumption is unsafe. JSTsfnSynchronizationContext.Post checks IsDisposed and then calls _tsfn.NonBlockingCall, while Dispose can release the TSFN between those operations. Since reference finalizers now always post here during environment teardown, that race can call a released native TSFN. Gate in-flight calls and close the gate before releasing the TSFN.

        // The guard above handles an already-disposed context; if it is disposed concurrently after
        // that check, the posted delete is still a safe no-op (the napi_ref went with the env).

src/NodeApi/DotNetHost/NativeHost.cs:501

• On .NET Framework, the managed teardown notification uses _runtimeHost->ExecuteInDefaultAppDomain. This callback closes and nulls _runtimeHost without notifying the managed host, so the later environment finalizer skips its notification and leaks _addonGCHandle plus the managed context. Run the full idempotent Dispose() path so notification occurs before the CLR host is closed.

        exports.DefineProperties(new JSPropertyDescriptor(
            "dispose", (_) => { CloseRuntimeHost(); return default; }));

src/NodeApi.Generator/ModuleGenerator.cs:309

• This hosted-module scope inherits the ModuleHolder from the current ManagedHost.LoadModule callback. Consequently all dynamically loaded generated modules share one StrongBox; loading a second module overwrites the instance observed by callbacks from the first module. Create a fresh module holder at each module-initialization boundary while continuing to share the runtime context.

        s += "using var moduleScope = JSValueScope.CreateRuntimeScope(env);";
        s += $"return {ModuleExportsMethodName}(moduleScope, exports);";

src/NodeApi/Interop/JSRuntimeContext.cs:880

• Replacing an owning annotation of the same type drops the previous IDisposable without disposing it, even though this API transfers disposal responsibility to the context. Either reject duplicate keys or dispose the previous value when replacing it so owned resources are not leaked.

    public void SetDisposableAnnotation<T>(T value) where T : class, IDisposable
    {
        if (value is null) throw new ArgumentNullException(nameof(value));
        (_disposableAnnotations ??= new())[typeof(T)] = value;

[Copilot]

Pull request overview

Copilot reviewed 35 out of 35 changed files in this pull request and generated 4 comments.

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

src/NodeApi/DotNetHost/NativeHost.cs:501

• On .NET Framework this releases _runtimeHost, but environment teardown later requires that same pointer to call OnEnvironmentFinalize. After an explicit JS dispose(), the condition in NotifyManagedHostEnvironmentFinalize is false, so the managed registration GCHandle and context are never released. Use the full disposal path so the managed host is notified before the runtime-host pointer is cleared.

        // Define a dispose method implemented by the native host that closes the CLR context.
        // The managed host proxy will pass through dispose calls to this callback.
        exports.DefineProperties(new JSPropertyDescriptor(
            "dispose", (_) => { CloseRuntimeHost(); return default; }));

src/NodeApi/Interop/JSRuntimeContext.cs:291

• This unconditionally overwrites an occupied slot. A second JSRuntimeContext.Create for the same environment leaves the first context rooted by a GCHandle that is no longer reachable through instance data, so it is never disposed. Reject an already-populated slot (and free the newly allocated handle on registration failure) to enforce the stated one-context-per-env invariant.

        ((nint*)instanceData)[s_instanceDataSlot] = ContextHandle;

src/NodeApi/JSReference.cs:374

• The context can be disposed after the IsDisposed check but before this post. JSTsfnSynchronizationContext.Post itself also checks then calls NonBlockingCall, while Dispose can concurrently release the TSFN, producing a native use-after-release. A try/catch cannot protect that race; gate in-flight TSFN calls against release before using this path for cross-thread reference cleanup.

        if (disposing)
        {
            // Delete the reference on the JS thread (inline if already there).
            _context.SynchronizationContext.Post(
                () => runtime.DeleteReference(env, handle).ThrowIfFailed(), allowSync: true);

[Copilot]

Pull request overview

Copilot reviewed 35 out of 35 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 35 out of 35 changed files in this pull request and generated 2 comments.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/NodeApi/Interop/JSRuntimeContext.cs:1023

• These cleanup calls are now guaranteed not to release any native references: IsDisposed is set at line 1006, and JSReference.Dispose immediately returns when its owning context is disposed. This matters outside env finalization—for example, the exported host dispose() tears down the managed context while the napi_env remains alive—so every mapped napi_ref is retained until eventual env shutdown. The disposal API needs to distinguish live-env explicit teardown (delete references before closing the synchronization context) from env-finalizer teardown (where Node reclaims them and no Node-API call is allowed).

        try { DisposeReferences(_objectMap.Select((entry) => entry.Value)); }
        catch (Exception ex) { firstFailure ??= ExceptionDispatchInfo.Capture(ex); }

[Copilot]

Pull request overview

Copilot reviewed 35 out of 35 changed files in this pull request and generated 3 comments.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/NodeApi/JSReference.cs:136

• After the owning context is disposed, this still returns its already-created (and disposed) synchronization context. Run/Run<T> then call Send, which simply returns when disposed, so the documented action is never invoked and Run<T> silently returns default instead of throwing ObjectDisposedException. Check the reference/context state before exposing the synchronization context so these operations reach the same disposed guard as GetValue.

    public JSSynchronizationContext? SynchronizationContext => _context.SynchronizationContext;

[Copilot]

Pull request overview

Copilot reviewed 35 out of 35 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 35 out of 35 changed files in this pull request and generated 1 comment.

[Copilot]

🟡 Changes recommended

Deferred teardown can dispose the wrong context, and tracing can silently swallow callback failures.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/NodeApi/Runtime/TracingJSRuntime.cs:402

• The outer catch silently converts preprocessing/postprocessing failures into undefined when tracing is enabled. TraceCallback only catches exceptions from descriptor.Callback; GetDataAndLength, descriptor conversion, argument Format, and return Format all run outside that catch. Report those failures as JS exceptions while the scope is current, as the non-tracing callback path does, while retaining the outer catch for scope-disposal failures.

• Files reviewed: 35/35 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Unresolved registration rollback, unmanaged-boundary, and scope-disposal issues can break context resolution or leak native runtimes.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

docs/concepts/runtime-model.md:117

• This says the native reference is released when its context is disposed, but the implementation intentionally marks the context disposed before visiting references, causing JSReference.Dispose() to skip napi_delete_reference; on explicit context disposal, Node retains the reference until later environment teardown. Document that context disposal invalidates the managed reference and that the native napi_ref is reclaimed by environment teardown.

• Files reviewed: 35/35 changed files
• Comments generated: 4
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Deferred teardown can dispose a context while an earlier scope for that context remains open under alternating context nesting.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 35/35 changed files
• Comments generated: 1
• Review effort level: Balanced