Initialize linked modules within the current JS value scope

src/NodeApi/Runtime/NodeEmbedding.cs

@@ -433,7 +433,19 @@ internal static unsafe napi_value ModuleInitializeCallbackAdapter(
         nint module_name,
         napi_value exports)
     {
-        using var jsValueScope = new JSValueScope(JSValueScopeType.Root, env, JSRuntime);
+        // Linked modules are initialized lazily by process._linkedBinding(), so the runtime's
+        // scope is usually current on this thread. Node.js passes a separate napi_env for each
+        // module; nesting a root scope for it would fail or, on dispose, leave the thread
+        // without a current scope. Initialize the module within the runtime's scope instead,
+        // so that its exports share the runtime context and later callbacks into them work.
+        // A callback scope cannot be nested in a handle scope, so that is used as is.
+        using JSValueScope? jsValueScope = JSValueScope.CurrentOrNull?.ScopeType switch
+        {
+            null => new JSValueScope(JSValueScopeType.Root, env, JSRuntime),
+            JSValueScopeType.Handle or JSValueScopeType.Escapable => null,
+            _ => new JSValueScope(JSValueScopeType.Callback),
+        };
+        napi_env scopeEnv = JSValueScope.Current.UncheckedEnvironmentHandle;
         try
         {
             var callback = (InitializeModuleCallback)GCHandle.FromIntPtr(cb_data).Target!;
@@ -448,6 +460,19 @@ internal static unsafe napi_value ModuleInitializeCallbackAdapter(
             JSError.ThrowError(ex);
             return napi_value.Null;
         }
+        finally
+        {
+            // Pending exceptions are tracked per napi_env, and Node.js only reports those of the
+            // module's env to the caller of process._linkedBinding().
+            if (scopeEnv != env &&
+                JSRuntime.IsExceptionPending(scopeEnv, out bool isPending) == napi_status.napi_ok &&
+                isPending &&
+                JSRuntime.GetAndClearLastException(scopeEnv, out napi_value error) ==
+                    napi_status.napi_ok)
+            {
+                JSRuntime.Throw(env, error);
+            }
+        }
     }
 
 #if UNMANAGED_DELEGATES

src/NodeApi/Runtime/NodejsRuntime.Embedding.cs

@@ -681,7 +681,7 @@ public override NodeEmbeddingStatus EmbeddingRuntimeConfigAddModule(
         node_embedding_data_release_callback release_init_module_data,
         int module_node_api_version)
     {
-        PooledBuffer moduleNameBuffer = PooledBuffer.FromSpanUtf8(module_name);
+        using PooledBuffer moduleNameBuffer = PooledBuffer.FromSpanUtf8(module_name);
         fixed (byte* moduleNamePtr = moduleNameBuffer)
             return Import(ref node_embedding_runtime_config_add_module)(
                 runtime_config,

src/NodeApi/Runtime/PooledBuffer.cs

@@ -68,33 +68,42 @@ public void Dispose()
     [EditorBrowsable(EditorBrowsableState.Never)]
     public readonly ref byte GetPinnableReference() => ref Span.GetPinnableReference();
 
+    // Native callers may read these buffers as null-terminated C strings. A null string
+    // produces an empty buffer (null pointer); an empty string produces just the terminator.
     public static unsafe PooledBuffer FromStringUtf8(string? value)
     {
-        if (string.IsNullOrEmpty(value))
+        if (value == null)
         {
             return Empty;
         }
 
         int byteLength = Encoding.UTF8.GetByteCount(value);
         PooledBuffer buffer = new(byteLength, byteLength + 1);
-        Encoding.UTF8.GetBytes(value, 0, value!.Length, buffer.Buffer, 0);
+        Encoding.UTF8.GetBytes(value, 0, value.Length, buffer.Buffer, 0);
 
+        // Pooled arrays are not zeroed.
+        buffer.Buffer[byteLength] = 0;
         return buffer;
     }
 
     public static unsafe PooledBuffer FromSpanUtf8(ReadOnlySpan<char> value)
     {
         if (value.IsEmpty)
         {
-            return Empty;
+            PooledBuffer emptyString = new(0, 1);
+            emptyString.Buffer[0] = 0;
+            return emptyString;
         }
 
         fixed (char* valuePtr = value)
         {
             int byteLength = Encoding.UTF8.GetByteCount(valuePtr, value.Length);
             PooledBuffer buffer = new(byteLength, byteLength + 1);
             fixed (byte* bufferPtr = buffer.Span)
-                Encoding.UTF8.GetBytes(valuePtr, value.Length, bufferPtr, byteLength + 1);
+                Encoding.UTF8.GetBytes(valuePtr, value.Length, bufferPtr, byteLength);
+
+            // Pooled arrays are not zeroed.
+            buffer.Buffer[byteLength] = 0;
             return buffer;
         }
     }

src/NodeApi/Runtime/Utf8StringArray.cs

@@ -47,8 +47,10 @@ public unsafe Utf8StringArray(ReadOnlySpan<string> strings)
             {
                 Utf8Strings[i] = stringBufferPtr + offset;
                 offset += Encoding.UTF8.GetBytes(
-                    src, strings[i].Length, (byte*)(stringBufferPtr + offset), byteLength - offset)
-                    + 1; // +1 for the string Null-terminator.
+                    src, strings[i].Length, (byte*)(stringBufferPtr + offset), byteLength - offset);
+
+                // Pooled arrays are not zeroed.
+                _stringBuffer[offset++] = 0;
             }
         }
     }

test/NodejsEmbeddingTests.cs

@@ -4,6 +4,7 @@
 #pragma warning disable CA1822 // Mark members as static
 
 using System;
+using System.Collections.Generic;
 using System.IO;
 using System.Linq;
 using System.Text;
@@ -260,6 +261,183 @@ public void ErrorPropagation()
                 $"at {typeof(NodejsEmbeddingTests).FullName}.", StringComparison.Ordinal));
     }
 
+    [Theory]
+    [InlineData("x")]
+    [InlineData("")]
+    public void ThrowErrorFromCallback(string message)
+    {
+        using NodeEmbeddingThreadRuntime nodejs = CreateNodeEmbeddingThreadRuntime();
+
+        nodejs.Run(() =>
+        {
+            JSValue.Global.SetProperty("throwError", JSValue.CreateFunction("throwError", (args) =>
+            {
+                // Rented buffers that are not zeroed must not leak into the error message.
+                FillArrayPoolWithGarbage();
+                JSError.ThrowError((string)args[0]);
+                return JSValue.Undefined;
+            }));
+            JSValue.Global.SetProperty("message", message);
+
+            JSValue result = JSValue.RunScript("""
+                (() => {
+                    try { throwError(message); } catch (e) { return e.message; }
+                    return 'not thrown';
+                })()
+                """);
+            Assert.Equal(message, (string)result);
+        });
+    }
+
+    private static NodeEmbeddingThreadRuntime CreateRuntimeWithLinkedModule(
+        string mainScript = "",
+        NodeEmbedding.InitializeModuleCallback? onInitialize = null)
+    {
+        NodeEmbeddingModuleInfo linkedModule = new()
+        {
+            Name = "test-linked",
+            NodeApiVersion = NodeEmbedding.NodeApiVersion,
+            OnInitialize = onInitialize ?? ((runtime, moduleName, exports) =>
+            {
+                exports.SetProperty("moduleName", moduleName);
+                exports.SetProperty("add", JSValue.CreateFunction(
+                    "add", (args) => (int)args[0] + (int)args[1]));
+                return exports;
+            }),
+        };
+
+        // Modules are registered on the runtime thread while it is configured, so dirty the
+        // array pool lazily from there, right before the module name is marshalled.
+        static IEnumerable<NodeEmbeddingModuleInfo> WithGarbageInArrayPool(
+            NodeEmbeddingModuleInfo module)
+        {
+            FillArrayPoolWithGarbage();
+            yield return module;
+        }
+
+        return NodejsPlatform.CreateThreadRuntime(
+            Path.Combine(GetRepoRootDirectory(), "test"),
+            new NodeEmbeddingRuntimeSettings
+            {
+                MainScript = MainScript + mainScript,
+                Modules = WithGarbageInArrayPool(linkedModule),
+            });
+    }
+
+    private static void AssertJSThreadIsUsable(NodeEmbeddingThreadRuntime nodejs)
+    {
+        nodejs.Run(() =>
+        {
+            Assert.Equal(3, (int)JSValue.RunScript("1 + 2"));
+        });
+    }
+
+    [Fact]
+    public void LinkedModuleFromScript()
+    {
+        using NodeEmbeddingThreadRuntime nodejs = CreateRuntimeWithLinkedModule();
+
+        nodejs.Run(() =>
+        {
+            JSValue linked = JSValue.RunScript("process._linkedBinding('test-linked')");
+            Assert.Equal("test-linked", (string)linked["moduleName"]);
+            Assert.Equal(3, (int)linked.CallMethod("add", 1, 2));
+
+            // The scope that was current before the module was initialized must be restored.
+            Assert.Equal(3, (int)JSValue.RunScript("1 + 2"));
+        });
+
+        AssertJSThreadIsUsable(nodejs);
+        nodejs.Dispose();
+        Assert.Equal(0, nodejs.ExitCode);
+    }
+
+    [Fact]
+    public void LinkedModuleFromMainScript()
+    {
+        using NodeEmbeddingThreadRuntime nodejs = CreateRuntimeWithLinkedModule(
+            "globalThis.linked = process._linkedBinding('test-linked');\n");
+
+        nodejs.Run(() =>
+        {
+            Assert.Equal(3, (int)JSValue.RunScript("globalThis.linked.add(1, 2)"));
+        });
+
+        AssertJSThreadIsUsable(nodejs);
+        nodejs.Dispose();
+        Assert.Equal(0, nodejs.ExitCode);
+    }
+
+    [Fact]
+    public async Task LinkedModuleFromESModule()
+    {
+        using NodeEmbeddingThreadRuntime nodejs = CreateRuntimeWithLinkedModule();
+
+        int result = await nodejs.RunAsync(async () =>
+        {
+            JSValue testModule = await nodejs.ImportAsync(
+                "./test-linked-module.mjs", null, esModule: true);
+            return (int)testModule["result"];
+        });
+        Assert.Equal(3, result);
+
+        AssertJSThreadIsUsable(nodejs);
+        nodejs.Dispose();
+        Assert.Equal(0, nodejs.ExitCode);
+    }
+
+    [Fact]
+    public void LinkedModuleFromHandleScope()
+    {
+        using NodeEmbeddingThreadRuntime nodejs = CreateRuntimeWithLinkedModule();
+
+        nodejs.Run(() =>
+        {
+            using JSValueScope handleScope = new();
+            Assert.Equal(
+                "test-linked",
+                (string)JSValue.RunScript("process._linkedBinding('test-linked').moduleName"));
+        });
+
+        AssertJSThreadIsUsable(nodejs);
+        nodejs.Dispose();
+        Assert.Equal(0, nodejs.ExitCode);
+    }
+
+    [Theory]
+    [InlineData(false)]
+    [InlineData(true)]
+    public void LinkedModuleInitializationError(bool throwJSError)
+    {
+        using NodeEmbeddingThreadRuntime nodejs = CreateRuntimeWithLinkedModule(
+            onInitialize: (runtime, moduleName, exports) =>
+            {
+                if (throwJSError)
+                {
+                    JSError.ThrowError("init failed");
+                    return exports;
+                }
+
+                throw new InvalidOperationException("init failed");
+            });
+
+        nodejs.Run(() =>
+        {
+            // The error must reach the caller of process._linkedBinding().
+            JSValue result = JSValue.RunScript("""
+                (() => {
+                    try { process._linkedBinding('test-linked'); } catch (e) { return e.message; }
+                    return 'not thrown';
+                })()
+                """);
+            Assert.Equal("init failed", (string)result);
+        });
+
+        AssertJSThreadIsUsable(nodejs);
+        nodejs.Dispose();
+        Assert.Equal(0, nodejs.ExitCode);
+    }
+
     [Fact]
     public async Task WorkerIsMainThread()
     {

test/TestUtils.cs

@@ -2,6 +2,10 @@
 // Licensed under the MIT License.
 
 using System;
+#if !NETFRAMEWORK
+using System.Buffers;
+#endif
+using System.Collections.Generic;
 using System.Diagnostics;
 using System.IO;
 using System.Runtime.InteropServices;
@@ -142,6 +146,31 @@ public static void CopyIfNewer(string sourceFilePath, string targetFilePath)
         }
     }
 
+    /// <summary>
+    /// Fills arrays in the shared byte array pool (as seen by the calling thread) with non-zero
+    /// bytes, so that code relying on rented buffers being zeroed fails deterministically.
+    /// </summary>
+    public static void FillArrayPoolWithGarbage()
+    {
+#if !NETFRAMEWORK
+        List<byte[]> rented = new();
+        for (int size = 1; size <= 1024; size *= 2)
+        {
+            for (int i = 0; i < 8; i++)
+            {
+                byte[] array = ArrayPool<byte>.Shared.Rent(size);
+                array.AsSpan().Fill(0xAA);
+                rented.Add(array);
+            }
+        }
+
+        foreach (byte[] array in rented)
+        {
+            ArrayPool<byte>.Shared.Return(array);
+        }
+#endif
+    }
+
     public static Task RunInThread(Action action)
     {
         TaskCompletionSource<bool> threadCompletion = new();