Initialize linked modules within the current JS value scope by r-Larch · Pull Request #512 · microsoft/node-api-dotnet

René Larch (r-Larch) · 2026-10-21T20:14:48Z

Fixes #464.

Stacked on #510. This branch includes #510's two commits, because the linked module's name is marshalled through PooledBuffer (without #510 the tests fail with No such binding was linked: test-linked). Only the last commit, "Initialize linked modules within the current JS value scope", belongs to this PR. I'll rebase once #510 is merged.

Calling process._linkedBinding() for a module registered through NodeEmbeddingRuntimeSettings.Modules failed or crashed the process:

Called from	Before
a script in `NodeEmbeddingThreadRuntime.Run`	`InvalidOperationException: A Root scope cannot be created within another scope.`
`MainScript`	thread left without a current scope → `JSInvalidThreadAccessException: There is no active JS value scope` (the stack overflow in the issue)
an ES module via `ImportAsync`	native assertion `(v8_scope_data_->nest_level()) == (nest_level)` in `CloseV8Scope`, exit code 134

Cause: ModuleInitializeCallbackAdapter always opened a new root scope for the napi_env that Node.js creates for each linked module (node_napi_env__::New in EmbeddedRuntime::RegisterModule). Linked modules are initialized lazily, while the runtime's scope is normally current on the JS thread. A nested root scope either throws, or drops its parent and leaves JSValueScope.Current null when it is disposed.

Fix: if a scope is current, open a callback scope within it (the same as for JS→.NET callbacks); otherwise fall back to a root scope as before. The module's exports then share the runtime's context (sync context, require, marshaller), and later calls from JS into the exported .NET functions find a matching parent scope. This is close to the workaround in the issue, which removes the scope entirely, but keeps a scope.

Note: draft PR #500 also rewrites this adapter (TryEnterRuntimeScope(env)) as part of a larger scope refactor. This PR is deliberately minimal so it can land on current main on its own. Happy to close it if #500 lands first and covers #464.

Tests

NodejsEmbeddingTests.LinkedModuleFromScript, LinkedModuleFromMainScript and LinkedModuleFromESModule (plus test/test-linked-module.mjs). Each one registers a linked module with a dirty array pool, calls the module, and then checks that the JS thread is still usable. All three fail on main; two of them crash the test host.

From review: LinkedModuleInitializationError checks that an error thrown by the initializer reaches the caller of _linkedBinding(). The adapter moves a pending exception from the runtime's env to the module's env. LinkedModuleFromHandleScope initializes a module while a handle scope is current.

Local run (Windows x64, .NET SDK 10.0.402): net10.0 160/162, net9.0 129/130, net8.0 129/130. The only failure, napi-dotnet/overloads, fails the same way on main. I could not run net472 locally.

🤖 Generated with Claude Code

The string buffer is rented from ArrayPool<byte>.Shared on .NET, which is not zeroed, so the byte reserved for each terminator could hold stale data and native code read argv entries past their end. Same fix as microsoft#482 (fixes microsoft#481); needed here because the new tests dirty the shared pool before the Node.js platform is created. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

PooledBuffer.FromStringUtf8 and FromSpanUtf8 reserved a byte for the terminator but never wrote it. On .NET the array comes from ArrayPool<byte>.Shared, which is not zeroed, so native functions that take a null-terminated const char* without a length read stale bytes past the string (or past the array): napi_throw_error / napi_throw_type_error / napi_throw_range_error / node_api_throw_syntax_error (code and msg), node_embedding_runtime_config_add_module and node_embedding_last_error_message_set. For example JSError.ThrowError("x") produced a JS error message of "x" followed by garbage, and a linked module could be registered under a corrupted name. - Write the terminator explicitly in both helpers. - Encode an empty string as just the terminator instead of returning PooledBuffer.Empty, which pinned to a null pointer (ThrowError("") passed msg = NULL, so napi_throw_error failed and nothing was thrown). A null string still maps to a null pointer. - Return the module-name buffer to the pool in EmbeddingRuntimeConfigAddModule (Node.js copies the name). Fixes microsoft#508. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Calling process._linkedBinding() for a module registered through NodeEmbeddingRuntimeSettings.Modules failed or crashed the process (microsoft#464): - From a script run via NodeEmbeddingThreadRuntime.Run: "A Root scope cannot be created within another scope." - From MainScript: the nested root scope dropped its parent on dispose, so the thread was left without a current scope ("There is no active JS value scope", reported as a stack overflow in the issue). - From an ES module imported via ImportAsync: native assertion "(v8_scope_data_->nest_level()) == (nest_level)" in CloseV8Scope. ModuleInitializeCallbackAdapter always opened a new root scope for the napi_env that Node.js creates for each linked module. Modules are initialized lazily, while the runtime's scope is normally current on the JS thread, so open a callback scope within it instead and only fall back to a root scope when no scope is current. The module's exports then share the runtime's context, and later calls from JS into the exported .NET callbacks find a matching parent scope. Fixes microsoft#464. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

azure-pipelines · 2026-10-21T20:14:59Z

Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run.

Copilot

🟡 Changes recommended

Handle-scope initialization and error forwarding to the module environment remain unresolved.

2 open issues

What changed in this PR

Fixes linked-module initialization by sharing the current JavaScript runtime context. Includes the UTF-8 buffer fixes stacked from #510.

Changes:

Uses callback scopes for linked-module initialization when a scope is active.
Explicitly terminates UTF-8 strings and disposes module-name buffers.
Adds buffer, error-propagation, and linked-module regression tests.

File	Description
test/Utf8BufferTests.cs	Tests UTF-8 termination and null handling.
test/TestUtils.cs	Adds a helper to dirty pooled buffers.
test/test-linked-module.mjs	Exercises linked binding from an ES module.
test/NodejsEmbeddingTests.cs	Adds error and linked-module integration tests.
src/NodeApi/Runtime/Utf8StringArray.cs	Writes string terminators explicitly.
src/NodeApi/Runtime/PooledBuffer.cs	Terminates UTF-8 buffers and distinguishes empty from null.
src/NodeApi/Runtime/NodejsRuntime.Embedding.cs	Disposes the module-name buffer.
src/NodeApi/Runtime/NodeEmbedding.cs	Initializes linked modules within the current context.

_{Review effort: Balanced}

Addresses review feedback on microsoft#512: - Node-API tracks pending exceptions per napi_env, and Node.js only reports those of the module's env to the caller of process._linkedBinding(). Errors thrown by the initializer within the runtime's scope were only logged as "Uncaught N-API callback exception" (DEP0168). Move a pending exception from the scope's env to the module's env before returning. - A callback scope cannot be nested in a handle or escapable scope, so initializing a module while one is current threw outside the adapter's try/catch. Use the current scope as is in that case. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot

🔵 Needs a closer look

Native scope lifetimes and cross-environment exception handling warrant human validation across supported runtimes.

No open issues 🎉

2 resolved since last review

_{Review effort: Balanced}

René Larch (r-Larch) and others added 3 commits October 21, 2026 21:01

Copilot AI balanced review requested due to automatic review settings October 21, 2026 20:14

Copilot started reviewing on behalf of René Larch (r-Larch) October 21, 2026 20:15

View session

Copilot AI reviewed Oct 21, 2026

View reviewed changes

Comment thread src/NodeApi/Runtime/NodeEmbedding.cs Outdated

Comment thread src/NodeApi/Runtime/NodeEmbedding.cs Outdated

Copilot AI balanced review requested due to automatic review settings October 21, 2026 20:31

Copilot started reviewing on behalf of René Larch (r-Larch) October 21, 2026 20:32

View session

Copilot AI reviewed Oct 21, 2026

View reviewed changes

Navigation Menu

Repository navigation

Initialize linked modules within the current JS value scope - #512

Initialize linked modules within the current JS value scope#512
René Larch (r-Larch) wants to merge 4 commits into
microsoft:mainfrom
r-Larch:fix/embedding-module-scope

René Larch (r-Larch) commented Oct 21, 2026 •

edited

Loading

Uh oh!

azure-pipelines Bot commented Oct 21, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

René Larch (r-Larch) commented Oct 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tests

Uh oh!

azure-pipelines Bot commented Oct 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

René Larch (r-Larch) commented Oct 21, 2026 •

edited

Loading