Fix worker_threads teardown crash (SIGSEGV) and hang when node-api-dotnet is loaded in a Worker

src/NodeApi/DotNetHost/NativeHost.cs

@@ -43,13 +43,155 @@ public static void Trace(string msg)
         }
     }
 
+    private static bool s_moduleUnloadPrevented;
+
+    /// <summary>
+    /// Pins this native host module in memory so the OS never unloads it.
+    /// </summary>
+    /// <remarks>
+    /// This native host is compiled with NativeAOT, so it embeds a .NET runtime whose
+    /// per-thread cleanup is registered with the OS via a <c>pthread_key</c> destructor that
+    /// points into this module's own code. Node.js unloads (<c>dlclose</c>) an addon when the
+    /// environment that loaded it is torn down. When a <c>worker_threads</c> Worker loads this
+    /// module and is then terminated, Node unloads the module while the worker's OS thread is
+    /// still alive; the still-registered destructor then points at unmapped memory and the
+    /// process crashes with SIGSEGV as the thread exits (glibc <c>__nptl_deallocate_tsd</c>).
+    /// Keeping the module mapped for the lifetime of the process keeps that destructor valid.
+    /// <para/>
+    /// This is scoped to Linux (glibc) and macOS, where the native host can be unloaded before
+    /// the worker thread exits; on Windows module/thread teardown does not hit this issue. The
+    /// pin is best-effort: any failure is traced but does not block init.
+    /// </remarks>
+    private static unsafe void PreventModuleUnload()
+    {
+        if (s_moduleUnloadPrevented)
+        {
+            return;
+        }
+
+        s_moduleUnloadPrevented = true;
+
+        bool isMacOS = RuntimeInformation.IsOSPlatform(OSPlatform.OSX);
+        if (!RuntimeInformation.IsOSPlatform(OSPlatform.Linux) && !isMacOS)
+        {
+            return;
+        }
+
+        try
+        {
+            // Resolve the file path of this shared library from the address of one of its
+            // own functions, then re-open it with RTLD_NODELETE so it is never unmapped.
+            nint moduleFunction =
+                (nint)(delegate* unmanaged[Cdecl]<napi_env, napi_value, napi_value>)
+                &InitializeModule;
+
+            nint fileName = default;
+            if (DlAddr(moduleFunction, out Dl_info info) != 0)
+            {
+                fileName = info.dli_fname;
+            }
+
+            if (fileName != default)
+            {
+                // RTLD_NOLOAD resolves the already-loaded module without loading a new copy;
+                // RTLD_NODELETE keeps it mapped for the process lifetime. The extra (never
+                // released) reference also prevents Node's dlclose from unmapping it.
+                const int RTLD_LAZY = 0x0001;
+                const int RTLD_NOLOAD_LINUX = 0x0004;
+                const int RTLD_NODELETE_LINUX = 0x1000;
+                const int RTLD_NOLOAD_MACOS = 0x0010;
+                const int RTLD_NODELETE_MACOS = 0x0080;
+                int flags = RTLD_LAZY | (isMacOS ?
+                    RTLD_NOLOAD_MACOS | RTLD_NODELETE_MACOS :
+                    RTLD_NOLOAD_LINUX | RTLD_NODELETE_LINUX);
+                nint handle = DlOpen(fileName, flags);
+                Trace($"    Pinned native host module ({(handle != default ? "ok" : "no-op")}).");
+            }
+            else
+            {
+                Trace("    Could not resolve native host module path to pin it.");
+            }
+        }
+        catch (Exception ex)
+        {
+            Trace("    Failed to pin native host module: " + ex);
+        }
+    }
+
+    // dladdr and dlopen are exported by libSystem on macOS. On Linux they are exported by
+    // libc.so.6 on glibc >= 2.34, but by libdl.so.2 on older glibc versions.
+    private static int DlAddr(nint addr, out Dl_info info)
+    {
+        if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
+        {
+            return DlAddrLibSystem(addr, out info);
+        }
+
+        try
+        {
+            return DlAddrLibc(addr, out info);
+        }
+        catch (Exception ex) when (ex is EntryPointNotFoundException or DllNotFoundException)
+        {
+            return DlAddrLibdl(addr, out info);
+        }
+    }
+
+    private static nint DlOpen(nint fileName, int flags)
+    {
+        if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
+        {
+            return DlOpenLibSystem(fileName, flags);
+        }
+
+        try
+        {
+            return DlOpenLibc(fileName, flags);
+        }
+        catch (Exception ex) when (ex is EntryPointNotFoundException or DllNotFoundException)
+        {
+            return DlOpenLibdl(fileName, flags);
+        }
+    }
+
+    [StructLayout(LayoutKind.Sequential)]
+    private struct Dl_info
+    {
+        public nint dli_fname;
+        public nint dli_fbase;
+        public nint dli_sname;
+        public nint dli_saddr;
+    }
+
+    [LibraryImport("libc.so.6", EntryPoint = "dladdr")]
+    private static partial int DlAddrLibc(nint addr, out Dl_info info);
+
+    [LibraryImport("libdl.so.2", EntryPoint = "dladdr")]
+    private static partial int DlAddrLibdl(nint addr, out Dl_info info);
+
+    [LibraryImport("libc.so.6", EntryPoint = "dlopen")]
+    private static partial nint DlOpenLibc(nint filename, int flags);
+
+    [LibraryImport("libdl.so.2", EntryPoint = "dlopen")]
+    private static partial nint DlOpenLibdl(nint filename, int flags);
+
+    [LibraryImport("/usr/lib/libSystem.B.dylib", EntryPoint = "dladdr")]
+    private static partial int DlAddrLibSystem(nint addr, out Dl_info info);
+
+    [LibraryImport("/usr/lib/libSystem.B.dylib", EntryPoint = "dlopen")]
+    private static partial nint DlOpenLibSystem(nint filename, int flags);
+
     [UnmanagedCallersOnly(
         EntryPoint = nameof(napi_register_module_v1),
         CallConvs = new[] { typeof(CallConvCdecl) })]
     public static napi_value InitializeModule(napi_env env, napi_value exports)
     {
         Trace($"> NativeHost.InitializeModule({env.Handle:X8}, {exports.Handle:X8})");
 
+        // Ensure this native module stays loaded for the lifetime of the process. See
+        // PreventModuleUnload() for details on the worker-thread teardown crash this avoids.
+        PreventModuleUnload();
+
         s_jsRuntime ??= new NodejsRuntime();
 
         // The native host JSValueScope is not disposed after a successful initialization. It

src/NodeApi/Interop/JSSynchronizationContext.cs

@@ -2,8 +2,13 @@
 // Licensed under the MIT License.
 
 using System;
+using System.Runtime.CompilerServices;
+using System.Runtime.InteropServices;
 using System.Threading;
 using System.Threading.Tasks;
+using Microsoft.JavaScript.NodeApi.Runtime;
+using static Microsoft.JavaScript.NodeApi.Runtime.JSRuntime;
+using static Microsoft.JavaScript.NodeApi.Runtime.NodejsRuntime;
 
 namespace Microsoft.JavaScript.NodeApi.Interop;
 
@@ -244,32 +249,85 @@ public Task<T> RunAsync<T>(Func<Task<T>> asyncAction)
     }
 }
 
-internal sealed class JSTsfnSynchronizationContext : JSSynchronizationContext
+internal sealed unsafe class JSTsfnSynchronizationContext : JSSynchronizationContext
 {
+    private readonly JSRuntime _runtime;
+    private readonly napi_env _env;
     private readonly JSThreadSafeFunction _tsfn;
+    private GCHandle _cleanupHandle;
 
     public JSTsfnSynchronizationContext()
     {
+        _runtime = JSValueScope.Current.Runtime;
+        _env = (napi_env)JSValueScope.Current;
         _tsfn = new JSThreadSafeFunction(
             maxQueueSize: 0,
             initialThreadCount: 1,
             asyncResourceName: (JSValue)nameof(JSSynchronizationContext));
 
         // Unref TSFN to indicate that this TSFN is not preventing Node.JS shutdown.
         _tsfn.Unref();
+
+        // Node runs environment cleanup hooks in reverse registration order. Registering this
+        // after the TSFN ensures it is released before Node closes the TSFN's libuv handle.
+        _cleanupHandle = GCHandle.Alloc(this);
+        napi_status status = _runtime.AddEnvCleanupHook(
+            _env,
+            new napi_cleanup_hook(s_cleanup),
+            (nint)_cleanupHandle);
+        if (status != napi_status.napi_ok)
+        {
+            _cleanupHandle.Free();
+            _tsfn.Release();
+            status.ThrowIfFailed();
+        }
     }
 
     public override void Dispose()
     {
         if (IsDisposed) return;
 
+        if (_cleanupHandle.IsAllocated)
+        {
+            _runtime.RemoveEnvCleanupHook(
+                _env,
+                new napi_cleanup_hook(s_cleanup),
+                (nint)_cleanupHandle).ThrowIfFailed();
+            _cleanupHandle.Free();
+        }
+
         base.Dispose();
 
         // Destroy TSFN by releasing last thread use count.
         // TSFN is deleted after this point and must not be used.
         _tsfn.Release();
     }
 
+#if !UNMANAGED_DELEGATES
+    private static readonly napi_cleanup_hook.Delegate s_cleanup = Cleanup;
+#else
+    private static readonly unsafe delegate* unmanaged[Cdecl]<nint, void> s_cleanup = &Cleanup;
+#endif
+
+#if UNMANAGED_DELEGATES
+    [UnmanagedCallersOnly(CallConvs = new[] { typeof(CallConvCdecl) })]
+#endif
+    private static unsafe void Cleanup(nint data)
+    {
+        GCHandle cleanupHandle = GCHandle.FromIntPtr(data);
+        JSTsfnSynchronizationContext context =
+            (JSTsfnSynchronizationContext)cleanupHandle.Target!;
+        context._cleanupHandle = default;
+        try
+        {
+            context.Dispose();
+        }
+        finally
+        {
+            cleanupHandle.Free();
+        }
+    }
+
     /// <summary>
     /// Increment reference count for the main loop async resource.
     /// Non-zero count prevents Node.JS process from exiting.

test/NativeAotTests.cs

@@ -18,6 +18,12 @@ public class NativeAotTests
 
     public static IEnumerable<object[]> TestCases { get; } = ListTestCases((testCaseName) =>
         !testCaseName.Contains("/dynamic_") &&
+
+        // The worker_teardown case validates NativeHost.PreventModuleUnload(), which only runs
+        // when the hosted host module (Microsoft.JavaScript.NodeApi.node) is loaded. A generated
+        // NativeAOT module has its own entry point that never calls it, so this case is only
+        // meaningful under the hosted CLR host. See HostedClrTests.
+        !testCaseName.Contains("/worker_teardown") &&
         !testCaseName.StartsWith("projects/", StringComparison.Ordinal));
 
     [Theory]

test/TestCases/napi-dotnet/worker_teardown.js

@@ -0,0 +1,43 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+// Regression test for a worker_threads teardown crash.
+//
+// When the native host is loaded ONLY inside a Worker (so no other reference keeps the
+// module mapped) and the Worker is then terminated, Node.js unloads (dlclose) the addon
+// while the worker's OS thread is still exiting. For a NativeAOT host that leaves a
+// dangling pthread-key destructor, so the process crashes with SIGSEGV as the thread exits.
+// NativeHost.PreventModuleUnload() pins the module to prevent that. This test fails (the
+// child node process exits non-zero) if the crash regresses.
+//
+// This validates the hosted host module (Microsoft.JavaScript.NodeApi.node), which is what
+// PreventModuleUnload() pins, so it runs under HostedClrTests only (excluded from
+// NativeAotTests, whose generated module has a separate entry point).
+//
+// The binding is intentionally NOT loaded on the main thread: doing so would keep another
+// module reference alive and mask the unload crash (which is why multi_instance.js cannot
+// cover this case).
+
+const assert = require('assert');
+const { Worker, isMainThread, parentPort } = require('worker_threads');
+
+if (isMainThread) {
+  const worker = new Worker(__filename);
+  worker.on('error', (err) => { throw err; });
+  worker.once('message', (message) => {
+    assert.strictEqual(message, 'ready');
+    // Let the worker settle, then tear it down. An unfixed host crashes during the
+    // worker thread's teardown after the module is unloaded.
+    setTimeout(async () => {
+      await worker.terminate();
+      // Keep the process alive briefly so any teardown crash surfaces as a non-zero
+      // exit code instead of being skipped by an immediate process exit.
+      setTimeout(() => process.exit(0), 300);
+    }, 300);
+  });
+} else {
+  // Load the native host ONLY in the worker.
+  const binding = require('../common').binding;
+  assert.ok(binding);
+  parentPort.postMessage('ready');
+}