Free the env instance-data block at teardown; fix module-disposable dedup and failed managed-init leak

- FinalizeInstanceData frees the per-env instance-data block once the last context on the env is disposed, instead of retaining it for the process lifetime; a host context's disposal cascades synchronously to the other slot, and the block is not nulled via napi_set_instance_data (that would double-free Node's finalizer record). Updates runtime-model.md.

- AddModuleDisposable deduplicates by reference identity rather than Equals, so equal-but-distinct IDisposable module instances are each disposed once. Adds a regression test.

- ManagedHost disposes its JSRuntimeContext on the failed-initialization path so its instance-data GCHandle, synchronization context, and resolve handlers are released when no teardown handshake was established.

Author: vmoroz

docs/concepts/runtime-model.md

@@ -75,9 +75,13 @@ contexts share an env. A runtime **reads and writes only its own slot**, so it n
 how callback dispatch and finalizers recover the context when no scope is yet current on the thread.
 
 At environment teardown the instance-data finalizer disposes the owning context, which **clears its
-slot and frees the rooting `GCHandle`**. The instance-data block itself is intentionally *not*
-freed: a wrapped-object finalizer may still run afterward and call `FromEnv`, and reading a freed
-block would be a use-after-free — whereas reading a **cleared slot** simply resolves to "no context."
+slot and frees the rooting `GCHandle`**. Disposing a host context cascades synchronously to the
+other slot's context, so once every context on the env is gone the finalizer **frees the block**.
+Freeing it there is no less safe than keeping it: a finalizer that called `FromEnv` after the
+instance-data finalizer would already be reading Node's own freed finalizer record (Node does not
+null its instance-data pointer), so retaining the block never protected that case. The block is not
+nulled out via `napi_set_instance_data` — that would delete the finalizer record Node is running and
+then double-free it.
 
 ## JavaScript value scopes
 
@@ -121,9 +125,9 @@ follows two rules:
 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
    cleared at teardown), the finalizer only frees its own native handle and does no JS work.
-2. **Never assume ordering** between an individual wrapped-object finalizer and the instance-data
-   finalizer. Node drains its pending finalizers without a guaranteed order relative to instance-data
-   finalization, which is exactly why the context block is retained after its slot is cleared.
+2. **Never assume ordering** among the env's finalizers. Node drains wrapped-object finalizers in no
+   guaranteed order, so a finalizer must tolerate the context's slot already being cleared (rule 1).
+   The instance-data finalizer frees the block only after every context on the env is disposed.
 
 ## See also
 

src/NodeApi.DotNetHost/ManagedHost.cs

@@ -255,7 +255,17 @@ public static unsafe napi_value InitializeModule(
         catch (Exception ex)
         {
             Trace($"Failed to load CLR managed host module: {ex}");
-            JSError.ThrowError(ex);
+            try
+            {
+                JSError.ThrowError(ex);
+            }
+            finally
+            {
+                // Failed init: nothing else disposes this context (when hosted, the native host
+                // never received a teardown callback), so dispose it here to release its slot
+                // GCHandle, synchronization context, and resolve handlers.
+                context.Dispose();
+            }
         }
 
 #if NETFRAMEWORK || NETSTANDARD

src/NodeApi/Interop/JSRuntimeContext.cs

@@ -272,9 +272,8 @@ private unsafe void RegisterInstanceData(napi_env env, JSRuntime runtime)
         runtime.GetInstanceData(env, out nint instanceData).ThrowIfFailed();
         if (instanceData == default)
         {
-            // Retained for the env's lifetime, never freed here or by the finalizer: a late
-            // wrapped-object finalizer may still read a cleared slot via FromEnv after teardown
-            // (see FinalizeInstanceData), so freeing this block would risk a use-after-free.
+            // One block per env, freed by FinalizeInstanceData when the last context on the env is
+            // disposed at teardown.
             instanceData = Marshal.AllocHGlobal(IntPtr.Size * InstanceDataSlotCount);
             for (int i = 0; i < InstanceDataSlotCount; i++)
             {
@@ -306,9 +305,7 @@ private static unsafe void FinalizeInstanceData(napi_env env, nint data, nint hi
         // Runs during env teardown, where calling into JS is forbidden. Dispose the owning
         // runtime's context (which clears its slot and frees its rooting GCHandle). Only this
         // runtime's slot is read, never the other runtime's (whose GCHandle belongs to a separate
-        // GC heap). The block is intentionally not freed: a wrapped-object finalizer may still run
-        // after this and resolve the context via FromEnv, which reads this block; a freed block
-        // would be a use-after-free, whereas a cleared slot safely resolves to no context.
+        // GC heap).
         nint slotHandle = ((nint*)data)[s_instanceDataSlot];
         if (slotHandle != default)
         {
@@ -321,6 +318,20 @@ private static unsafe void FinalizeInstanceData(napi_env env, nint data, nint hi
                 // A finalizer must never throw; teardown continues regardless.
             }
         }
+
+        // Free the shared block once the last context on the env is gone (all slots cleared);
+        // disposing a host context cascades synchronously to the other slot. Do not null it out
+        // via napi_set_instance_data: that deletes this very TrackedFinalizer, which Node then
+        // deletes again (double free).
+        for (int i = 0; i < InstanceDataSlotCount; i++)
+        {
+            if (((nint*)data)[i] != default)
+            {
+                return;
+            }
+        }
+
+        Marshal.FreeHGlobal(data);
     }
 
     /// <summary>
@@ -912,10 +923,18 @@ internal void AddModuleDisposable(IDisposable disposable)
     {
         if (disposable is null) throw new ArgumentNullException(nameof(disposable));
         _moduleDisposables ??= new();
-        if (!_moduleDisposables.Contains(disposable))
+
+        // Dedupe by identity, not Equals: a module class may override equality, but each distinct
+        // instance must be disposed once.
+        foreach (IDisposable existing in _moduleDisposables)
         {
-            _moduleDisposables.Add(disposable);
+            if (ReferenceEquals(existing, disposable))
+            {
+                return;
+            }
         }
+
+        _moduleDisposables.Add(disposable);
     }
 
     public void Dispose()
@@ -969,10 +988,9 @@ public void Dispose()
             }
         }
 
-        // Remove this context's root so it can be collected: clear its instance-data slot (a late
-        // wrapped-object finalizer then resolves no context via FromEnv and frees only its own
-        // handle) and free the rooting GCHandle. The block is left allocated (see
-        // FinalizeInstanceData) so a late FromEnv reads a cleared slot rather than freed memory.
+        // Remove this context's root so it can be collected: clear its instance-data slot (a
+        // concurrent FromEnv then resolves no context) and free the rooting GCHandle. The shared
+        // block itself is freed by FinalizeInstanceData once every context on the env is gone.
         if (ContextHandle != default)
         {
             Runtime.GetInstanceData(UncheckedEnvironmentHandle, out nint instanceData);

test/JSValueScopeTests.cs

@@ -146,6 +146,40 @@ public void DisposableModulesShareContextAndAreDisposedOnceAtTeardown()
         Assert.Equal(1, moduleB.DisposeCount);
     }
 
+    private sealed class EqualDisposable : IDisposable
+    {
+        public int DisposeCount { get; private set; }
+
+        public void Dispose() => DisposeCount++;
+
+        // All instances compare equal, to prove module disposables dedupe by identity, not Equals.
+        public override bool Equals(object? obj) => obj is EqualDisposable;
+
+        public override int GetHashCode() => 0;
+    }
+
+    [Fact]
+    public void ModuleDisposablesAreDedupedByIdentityNotEquality()
+    {
+        var moduleA = new EqualDisposable();
+        var moduleB = new EqualDisposable();
+        Assert.True(moduleA.Equals(moduleB));
+
+        napi_env env = new(Environment.CurrentManagedThreadId);
+        var context = new JSRuntimeContext(
+            env, _mockRuntime, new MockJSRuntime.SynchronizationContext());
+
+        context.AddModuleDisposable(moduleA);
+        context.AddModuleDisposable(moduleB);
+        context.AddModuleDisposable(moduleA); // Re-adding the same instance is a no-op.
+
+        context.Dispose();
+
+        // Both distinct instances are disposed once, despite comparing equal.
+        Assert.Equal(1, moduleA.DisposeCount);
+        Assert.Equal(1, moduleB.DisposeCount);
+    }
+
     [Fact]
     public void AccessValueFromClosedScope()
     {