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 2 comments.

Suppressed comments (2)

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

test/TestCases/napi-dotnet/worker_teardown_stress.js:35

• This one-shot error listener is tied to the readiness promise. After 'ready' resolves that promise, a worker error emitted during terminate() calls reject on an already-settled promise and is silently consumed, allowing this teardown regression test to pass. Keep an error handler that fails the test for the worker's full lifetime, as worker_teardown.js does.

src/NodeApi.DotNetHost/ManagedHost.cs:240

• The host becomes an owned annotation only after its constructor returns, but that constructor subscribes process-wide resolving handlers before several Node-API operations that can throw (ManagedHost.cs:85-135). If construction fails, the catch disposes a context that does not know about the partially constructed host, leaving those handlers subscribed and rooting the failed worker host. Make construction transactional by unsubscribing on constructor failure, or defer the subscriptions until all fallible initialization is complete.

            // Dispose the host with its environment: as a disposable annotation on the context, the
            // host's full Dispose (which unsubscribes the process-wide resolve handlers) runs when
            // the context is disposed at environment teardown. Mirrors the native host.
            context.SetDisposableAnnotation(host);

[Copilot]

Pull request overview

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

Suppressed comments (1)

src/NodeApi/DotNetHost/NativeHost.cs:501

• The JS dispose hook now closes _runtimeHost without notifying the managed host. On .NET Framework, _onEnvFinalize is intentionally unset and teardown can notify managed code only through _runtimeHost; after this callback nulls that pointer, NotifyManagedHostEnvironmentFinalize() skips the notification, so the registration GCHandle, managed JSRuntimeContext, and resolve handlers remain rooted. Run the full idempotent host disposal here so the managed registration is released before the runtime-host channel is closed.

            "dispose", (_) => { CloseRuntimeHost(); return default; }));

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

Suppressed comments (2)

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

src/NodeApi/DotNetHost/NativeHost.cs:559

• Explicit exports.dispose() can run while the environment remains alive, but this drops the strong JSReference without releasing its napi_ref. Its finalizer runs off-thread and JSInlineSynchronizationContext intentionally drops that delete, so every dispose/reinitialize cycle leaks another reference until environment teardown. Explicitly dispose _exports before clearing it; when this method is called from environment teardown, the already-disposed context makes that disposal a safe no-op.

        _exports = null;

docs/concepts/runtime-model.md:123

• The blanket “no JS allowed” rule does not match the implementation: JSValue.CallFinalizeAction still opens a runtime scope and invokes the user-supplied finalizer action when the context is live (src/NodeApi/JSValue.cs:1331-1336). Either enforce this rule in that path or document the current exception/limitation, otherwise this foundational guide states an invariant contributors cannot rely on.

### Finalizers run during teardown — no JS allowed

A finalizer (for a wrapped .NET object, an external, or a reference) may run while the environment is
being torn down, where **calling into JavaScript is forbidden**. Finalizer code in this library
follows two rules:

[Copilot]

Pull request overview

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

Suppressed comments (2)

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

test/TestCases/napi-dotnet/worker_teardown_stress.js:35

• If the worker exits before posting ready without emitting an error, this promise never settles. A pending promise does not keep Node alive, so the test process can exit successfully after only the first failed iteration. Race readiness against both error and premature exit, and keep racing errors through terminate().

src/NodeApi.DotNetHost/ManagedHost.cs:216

• CreateRuntimeScope is outside the try, but it lazily constructs the TSFN-backed synchronization context and can throw if TSFN or cleanup-hook creation fails. In the hosted path the context has already rooted itself in the managed instance-data slot, while the native registration has not been filled yet, so the native finalizer cannot notify this runtime to dispose it; the exception can also escape the unmanaged entry point. Include scope creation in a failure path that disposes context and translates the exception without letting it cross the native boundary.

        JSRuntimeContext context = new(env, runtime);
        using JSValueScope scope = JSValueScope.CreateRuntimeScope(env, context);

[Copilot]

Pull request overview

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

Suppressed comments (4)

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

src/NodeApi/JSValueScope.cs:189

• CreateRuntimeScope() with neither a parent nor an explicit context reaches FromEnv(default). Once any context has initialized s_instanceDataRuntime, that calls napi_get_instance_data with a null napi_env, producing a native API failure instead of the factory's predictable “context could not be resolved” error (and relying on the runtime to tolerate a null env). Only attempt env lookup when env is non-null.

        // 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/Interop/JSRuntimeContext.cs:953

• The disposed check is not atomic with inserting the owned annotation. A concurrent Dispose() can set IsDisposed, observe the dictionary before this insertion, finish teardown, and then this method stores a value that will never be disposed; concurrent mutation can also race the disposal enumeration. Synchronize this method and the annotation-disposal section with the same gate (or enforce and validate thread affinity).

        if (IsDisposed) throw new ObjectDisposedException(nameof(JSRuntimeContext));

        _disposableAnnotations ??= new();

src/NodeApi/DotNetHost/NativeHost.cs:559

• If CloseRuntimeHost() throws (it explicitly throws for a failed hostfxr_close), the registration fields are never cleared and _exports is never released. After an explicit JS dispose(), the managed callback has already freed _addonGCHandle; environment teardown can then retry Dispose() and invoke that callback with the stale handle, causing an invalid/double GCHandle access. Clear the registration immediately after notification and place all remaining cleanup in finally blocks.

        NotifyManagedHostEnvironmentFinalize();
        CloseRuntimeHost();
        _addonGCHandle = default;
        _onEnvFinalize = default;

        // JSReference.Dispose no-ops once its context is disposed, so this frees the napi_ref only on
        // an explicit dispose() (env alive), never during env-teardown finalization.
        _exports?.Dispose();
        _exports = null;

docs/concepts/runtime-model.md:127

• This states that reference finalizers resolve their context via FromEnv, but JSReference deliberately retains its owning _context and posts cleanup through that object; only the JSValue native finalizers were changed to use FromEnv. Since this page is presented as the authoritative teardown model, distinguish reference finalization from wrapped-object/external/action finalizers so contributors do not implement the wrong ownership pattern.

1. **Resolve the context from the env**, via `JSRuntimeContext.FromEnv(env)` — never by dereferencing
   a finalize hint that may already be freed. If `FromEnv` returns no live context (the slot was

[Copilot]

Pull request overview

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