Improve object safety

Directory.Packages.props

@@ -11,7 +11,7 @@
     <PackageVersion Include="Microsoft.NET.Test.Sdk" Version="17.5.0" />
     <PackageVersion Include="Microsoft.SourceLink.GitHub" Version="8.0.0" />
     <PackageVersion Include="Nerdbank.GitVersioning" Version="3.10.91" />
-    <PackageVersion Include="Nullability.Source" Version="2.1.0" />
+    <PackageVersion Include="Nullability.Source" Version="2.3.0" />
     <PackageVersion Include="System.Memory" Version="4.5.5" />
     <PackageVersion Include="System.Reflection.Emit" Version="4.7.0" />
     <PackageVersion Include="System.Reflection.MetadataLoadContext" Version="6.0.0" />

bench/Benchmarks.cs

@@ -144,7 +144,7 @@ protected void Setup()
         _reference = new JSReference(_jsFunction);
     }
 
-    private static JSValueScope NewJSScope() => new(JSValueScopeType.Callback);
+    private static JSValueScope NewJSScope() => JSValueScope.CreateRuntimeScope();
 
     // Benchmarks in the base class run in both CLR and AOT environments.
 

docs/features/js-value-scopes.md

@@ -7,9 +7,8 @@ A value is only valid within its scope; if the scope is closed (disposed), then
 access or use the value will throw
 [`JSValueScopeClosedException`](../reference/dotnet/Microsoft.JavaScript.NodeApi/JSValueScopeClosedException).
 
-Values received by a .NET method that is a JS callback are associated with a `Callback`
-[scope type](../reference/dotnet/Microsoft.JavaScript.NodeApi/JSValueScopeType). When the method
-returns, the callback scope is closed and any values in that scope become invalid.
+Values received by a .NET method that is a JS callback belong to the current scope for that call.
+When the method returns, that scope is closed and any values in it become invalid.
 
 ## Nesting and escaping scopes
 
@@ -23,7 +22,7 @@ JSFunction jsFunction = …
 
 foreach (string item in array)
 {
-    using (var nestedScope = new JSValueScope())
+    using (var nestedScope = JSValueScope.CreateHandleScope())
     {
         // Passing a .NET string to JS requires converting it to JSValue.
         // The conversion is implicit; the explicit cast is for illustration.
@@ -44,7 +43,7 @@ public JSValue EscapableScopeExample(JSCallbackArgs args)
 
     foreach (string item in array)
     {
-        using (var escapableScope = new JSValueScope(JSValueScopeType.Escapable))
+        using (var escapableScope = JSValueScope.CreateEscapableScope())
         {
             JSValue result = jsFunction.Call(thisArg: default, (JSValue)item);
             if (!result.IsUndefined())

examples/hermes-engine/HermesRuntime.cs

@@ -30,7 +30,8 @@ private HermesRuntime(JSDispatcherQueue dispatcherQueue)
         JSRuntime runtime = HermesApi.Load("hermes.dll");
         using HermesConfig tempConfig = new();
         hermes_create_runtime((hermes_config)tempConfig, out _runtime).ThrowIfFailed();
-        _rootScope = new JSValueScope(JSValueScopeType.Root, (napi_env)this, runtime);
+        JSRuntimeContext context = JSRuntimeContext.Create((napi_env)this, runtime);
+        _rootScope = JSValueScope.CreateRuntimeScope((napi_env)this, context);
         CreatePolyfills();
     }
 
@@ -98,7 +99,7 @@ public static explicit operator napi_env(HermesRuntime value)
     private void CreatePolyfills()
     {
         VerifyElseThrow(JSDispatcherQueue.GetForCurrentThread() == _dispatcherQueue);
-        using var scope = new JSValueScope();
+        using var scope = JSValueScope.CreateHandleScope();
 
         // Add global
         JSValue global = JSValue.Global;

src/NodeApi.DotNetHost/JSMarshaller.cs

@@ -89,9 +89,9 @@ public JSMarshaller()
         typeof(JSRuntimeContext).GetStaticProperty(nameof(JSRuntimeContext.Current))
             ?? throw new NotImplementedException("JSRuntimeContext.Current");
 
-    private static readonly PropertyInfo s_moduleContext =
-        typeof(JSModuleContext).GetStaticProperty(nameof(JSModuleContext.Current))
-            ?? throw new NotImplementedException("JSModuleContext.Current");
+    private static readonly PropertyInfo s_currentScope =
+        typeof(JSValueScope).GetStaticProperty(nameof(JSValueScope.Current))
+            ?? throw new NotImplementedException("JSValueScope.Current");
 
     private static readonly PropertyInfo s_valueItem =
         typeof(JSValue).GetIndexer(typeof(string))
@@ -1878,22 +1878,22 @@ private IEnumerable<Expression> BuildThisArgumentExpressions(
 
         if (type.GetCustomAttributes<JSModuleAttribute>().Any())
         {
-            // For a method on a module class, the .NET object is stored in the module context.
+            // For a method on a module class, the .NET object is the current module instance.
             // `ThisArg` is ignored for module-level methods.
 
             /*
-             * ObjectType? __this = JSRuntimeContext.Current.Module as ObjectType;
+             * ObjectType? __this = JSValueScope.Current.Module as ObjectType;
              * if (__this == null) return JSValue.Undefined;
              */
 
-            PropertyInfo moduleProperty = typeof(JSModuleContext).GetProperty(
-                nameof(JSModuleContext.Module))
-                ?? throw new NotImplementedException("JSModuleContext.Module");
+            PropertyInfo moduleProperty = typeof(JSValueScope).GetProperty(
+                nameof(JSValueScope.Module))
+                ?? throw new NotImplementedException("JSValueScope.Module");
             yield return Expression.Assign(
                 thisVariable,
                 Expression.TypeAs(
                     Expression.Property(
-                        Expression.Property(null, s_moduleContext),
+                        Expression.Property(null, s_currentScope),
                         moduleProperty),
                     type));
             yield return Expression.IfThen(

src/NodeApi.DotNetHost/ManagedHost.cs

@@ -40,7 +40,7 @@ public sealed class ManagedHost : JSEventEmitter, IDisposable
     private readonly AssemblyLoadContext _loadContext = new(name: default);
 #endif
 
-    private JSValueScope? _rootScope;
+    private JSRuntimeContext? _context;
 
     /// <summary>
     /// Component that dynamically exports types from loaded assemblies.
@@ -177,10 +177,14 @@ public static unsafe int InitializeModule(string argument)
         napi_env env = new((nint)ulong.Parse(args[0], NumberStyles.HexNumber));
         napi_value exports = new((nint)ulong.Parse(args[1], NumberStyles.HexNumber));
         napi_value* pResult = (napi_value*)(nint)ulong.Parse(args[2], NumberStyles.HexNumber);
+        ManagedHostRegistration* registration = args.Length > 3 ?
+            (ManagedHostRegistration*)(nint)ulong.Parse(args[3], NumberStyles.HexNumber) : null;
 #else
     [UnmanagedCallersOnly(CallConvs = new[] { typeof(CallConvCdecl) })]
-    public static napi_value InitializeModule(napi_env env, napi_value exports)
+    public static unsafe napi_value InitializeModule(
+        napi_env env, napi_value exports, nint registrationPtr)
     {
+        ManagedHostRegistration* registration = (ManagedHostRegistration*)registrationPtr;
         Trace($"> ManagedHost.InitializeModule({env.Handle:X8})");
         Trace($"    .NET Runtime version: {Environment.Version}");
 #endif
@@ -198,7 +202,13 @@ public static napi_value InitializeModule(napi_env env, napi_value exports)
             runtime = new TracingJSRuntime(runtime, trace);
         }
 
-        JSValueScope scope = new(JSValueScopeType.Root, env, runtime);
+        // The managed host registers its context in the environment instance-data block (at the
+        // module slot). When hosted, the native host owns that block and its finalizer signals
+        // environment teardown, so the managed context is a non-owner: it writes its own slot but
+        // does not claim the finalizer, and is disposed via the registration notification below.
+        bool hosted = registration != null;
+        JSRuntimeContext context = new(env, runtime);
+        using JSValueScope scope = JSValueScope.CreateRuntimeScope(env, context);
 
         try
         {
@@ -219,9 +229,20 @@ public static napi_value InitializeModule(napi_env env, napi_value exports)
 
             ManagedHost host = new(exportsObject)
             {
-                _rootScope = scope
+                _context = context
             };
 
+            if (hosted)
+            {
+                // Root the managed host for the environment lifetime and give the native host a
+                // native callback to invoke at teardown (never a JS call -- see OnEnvironmentFinalize).
+                registration->AddonGCHandle = (nint)GCHandle.Alloc(host);
+#if !(NETFRAMEWORK || NETSTANDARD)
+                registration->OnEnvFinalize =
+                    (nint)(delegate* unmanaged[Cdecl]<nint, void>)&OnEnvironmentFinalize;
+#endif
+            }
+
             Trace("< ManagedHost.InitializeModule()");
         }
         catch (Exception ex)
@@ -238,6 +259,62 @@ public static napi_value InitializeModule(napi_env env, napi_value exports)
 #endif
     }
 
+#if !(NETFRAMEWORK || NETSTANDARD)
+    /// <summary>
+    /// Called natively by the native host when the environment is being torn down. Runs during
+    /// environment finalization where calling into JavaScript is forbidden, so it touches only
+    /// managed state.
+    /// </summary>
+    [UnmanagedCallersOnly(CallConvs = new[] { typeof(CallConvCdecl) })]
+    private static void OnEnvironmentFinalize(nint addon) => OnEnvironmentFinalizeCore(addon);
+#else
+    /// <summary>
+    /// Called by the native host (through the default AppDomain) when the environment is being
+    /// torn down. Runs during environment finalization where calling into JavaScript is forbidden,
+    /// so it touches only managed state.
+    /// </summary>
+    public static int OnEnvironmentFinalize(string argument)
+    {
+        OnEnvironmentFinalizeCore((nint)ulong.Parse(argument, NumberStyles.HexNumber));
+        return 0;
+    }
+#endif
+
+    private static void OnEnvironmentFinalizeCore(nint addon)
+    {
+        if (addon == default)
+        {
+            return;
+        }
+
+        GCHandle handle = GCHandle.FromIntPtr(addon);
+        try
+        {
+            (handle.Target as ManagedHost)?.DisposeOnEnvironmentFinalize();
+        }
+        catch (Exception ex)
+        {
+            Trace($"Failed to dispose managed host on environment finalize: {ex}");
+        }
+        finally
+        {
+            handle.Free();
+        }
+    }
+
+    /// <summary>
+    /// Disposes the runtime context in response to environment teardown. No JavaScript may be
+    /// called here; disposing the context marks it disposed (so any late cross-thread post becomes
+    /// a no-op) and frees its GC handles. The context's references are reclaimed by Node as the
+    /// environment is torn down.
+    /// </summary>
+    private void DisposeOnEnvironmentFinalize()
+    {
+        JSRuntimeContext? context = _context;
+        _context = null;
+        context?.Dispose();
+    }
+
     /// <summary>
     /// Resolve references to Node API and other assemblies that loaded assemblies depend on.
     /// </summary>
@@ -592,8 +669,8 @@ protected override void Dispose(bool disposing)
     {
         if (disposing)
         {
-            _rootScope?.Dispose();
-            _rootScope = null;
+            _context?.Dispose();
+            _context = null;
 
 #if NETFRAMEWORK || NETSTANDARD
             AppDomain.CurrentDomain.AssemblyResolve -= OnResolvingAssembly;

src/NodeApi.DotNetHost/ManagedHostRegistration.cs

@@ -0,0 +1,35 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+using System;
+using System.Runtime.InteropServices;
+
+namespace Microsoft.JavaScript.NodeApi.DotNetHost;
+
+/// <summary>
+/// Native handshake structure the managed host fills in at initialization, so the native host can
+/// keep the managed host alive for the environment lifetime and notify it when the environment is
+/// torn down.
+/// </summary>
+/// <remarks>
+/// The layout must exactly match the native host's own copy of this structure (in the NodeApi
+/// assembly). Both are two pointer-sized fields, passed by pointer across the native/managed
+/// boundary. The native host and managed host run in separate .NET runtimes, so the structure is
+/// defined independently in each and only its binary layout is shared.
+/// </remarks>
+[StructLayout(LayoutKind.Sequential)]
+internal struct ManagedHostRegistration
+{
+    /// <summary>
+    /// A strong <see cref="GCHandle"/> to the managed host, allocated and freed only by managed
+    /// code. The native host treats it as an opaque pointer.
+    /// </summary>
+    public nint AddonGCHandle;
+
+    /// <summary>
+    /// A native callback pointer (<c>delegate* unmanaged&lt;nint, void&gt;</c>) the native host
+    /// invokes at environment teardown, or default when the native host uses another channel
+    /// (the .NET Framework host invokes the finalize method through the default AppDomain instead).
+    /// </summary>
+    public nint OnEnvFinalize;
+}

src/NodeApi.Generator/ModuleGenerator.cs

@@ -24,6 +24,7 @@ public class ModuleGenerator : SourceGenerator, ISourceGenerator
 {
     private const string ModuleInitializerClassName = "Module";
     private const string ModuleInitializeMethodName = "Initialize";
+    private const string ModuleExportsMethodName = "InitializeExports";
     private const string ModuleRegisterFunctionName = "napi_register_module_v1";
 
     private readonly JSMarshaller _marshaller = new()
@@ -287,26 +288,36 @@ private SourceBuilder GenerateModuleInitializer(
         s += $"public static class {ModuleInitializerClassName}";
         s += "{";
 
-        // The module scope is not disposed after a successful initialization. It becomes
-        // the parent of callback scopes, allowing the JS runtime instance to be inherited.
-        s += "private static JSValueScope _moduleScope;";
-
-        // The unmanaged entrypoint is used only when the AOT-compiled module is loaded.
+        // The unmanaged entrypoint is used only when the AOT-compiled module is loaded. As the
+        // root it creates the runtime context; there is no host to resolve it from.
         s += "#if !NETFRAMEWORK";
         s += $"[UnmanagedCallersOnly(EntryPoint = \"{ModuleRegisterFunctionName}\")]";
         s += $"public static napi_value _{ModuleInitializeMethodName}(napi_env env, napi_value exports)";
-        s += $"{s.Indent}=> {ModuleInitializeMethodName}(env, exports);";
+        s += "{";
+        s += "JSRuntimeContext context = JSRuntimeContext.Create(env);";
+        s += "using var moduleScope = JSValueScope.CreateRuntimeScope(env, context);";
+        s += $"return {ModuleExportsMethodName}(moduleScope, exports);";
+        s += "}";
         s += "#endif";
         s++;
 
-        // The main initialization entrypoint is called by the `ManagedHost`, and by the unmanaged entrypoint.
+        // The main initialization entrypoint is called by the `ManagedHost` that loaded this
+        // module; the scope resolves the runtime context from that host.
         s += $"public static napi_value {ModuleInitializeMethodName}(napi_env env, napi_value exports)";
         s += "{";
-        s += "_moduleScope = new JSValueScope(JSValueScopeType.Module, env, runtime: default);";
+        s += "using var moduleScope = JSValueScope.CreateRuntimeScope(env);";
+        s += $"return {ModuleExportsMethodName}(moduleScope, exports);";
+        s += "}";
+        s++;
+
+        // The shared body builds the exports within the module scope opened by an entrypoint
+        // above; the scope stays alive through the catch so it can build the JS error.
+        s += $"private static napi_value {ModuleExportsMethodName}(JSValueScope moduleScope, napi_value exports)";
+        s += "{";
         s += "try";
         s += "{";
-        s += "JSRuntimeContext context = _moduleScope.RuntimeContext;";
-        s += "JSValue exportsValue = new(exports, _moduleScope);";
+        s += "JSRuntimeContext context = moduleScope.RuntimeContext;";
+        s += "JSValue exportsValue = new(exports, moduleScope);";
         s++;
 
         if (moduleInitializer is IMethodSymbol moduleInitializerMethod)
@@ -340,7 +351,6 @@ private SourceBuilder GenerateModuleInitializer(
         s += "{";
         s += "System.Console.Error.WriteLine($\"Failed to export module: {ex}\");";
         s += "JSError.ThrowError(ex);";
-        s += "_moduleScope.Dispose();";
         s += "return exports;";
         s += "}";
         s += "}";