microsoft / microsoft/node-api-dotnet

Embedding args occasionally get trailing garbage on .NET (missing null terminator on a pooled buffer)

Open Beginner friendly
#481 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C#
Stars
783
Forks
80
PR merge metrics
No merged PRs in 30d

Description

Passing Args to NodeEmbeddingPlatformSettings intermittently fails with

bad option: --disable-wasm-trap-handler<random leftover bytes>

The tail is different every time and looks like fragments of unrelated strings from elsewhere in the process (table names, event names, and so on). It works if you create the platform right at startup, but once the app has been running for a while it starts failing, which pointed at the option value picking up stale memory.

Versions: Microsoft.JavaScript.NodeApi 0.9.11 with Microsoft.JavaScript.LibNode 20.1800.215 (Node 20.18.0), on net8.0. Reproduced on both Windows x64 and Linux x64 — it is tied to the TFM, not the OS.

Cause

In src/NodeApi/Runtime/Utf8StringArray.cs:

#if NETFRAMEWORK || NETSTANDARD
    _stringBuffer = new byte[byteLength];                       // zeroed
#else
    _stringBuffer = ArrayPool<byte>.Shared.Rent(byteLength);    // NOT zeroed
#endif

...

offset += Encoding.UTF8.GetBytes(
              src, strings[i].Length,
              (byte*)(stringBufferPtr + offset), byteLength - offset)
          + 1; // +1 for the string Null-terminator.

The + 1 reserves a byte for the terminator, but nothing ever writes the \0. On the netfx/netstandard path this is harmless because new byte[] is zero-initialized, so the reserved byte is already 0. On the .NET path the buffer comes from ArrayPool<byte>.Shared, which returns it with its previous contents intact, so the terminator byte can hold any leftover value. Native node then reads each argv entry as a C string and reads past the end of the intended option into that stale data.

This also explains the timing. On a fresh process the pool is empty, so the first rent returns zeroed memory and everything works. After the app has used ArrayPool<byte>.Shared for anything else (JSON, IO, DB drivers, and so on), the buffer comes back dirty and the option gets a trailing tail.

Repro

The failure can be made deterministic by dirtying the relevant pool bucket first:

using System.Buffers;
using Microsoft.JavaScript.NodeApi.Runtime;

// byteLength for ["node","--disable-wasm-trap-handler"] is (4+1)+(27+1)=33 -> 64-byte bucket
var marker = System.Text.Encoding.ASCII.GetBytes("_LEAKED_MARKER_1234567890");
for (int i = 0; i < 64; i++)
{
    var buf = ArrayPool<byte>.Shared.Rent(33);
    for (int k = 0; k < buf.Length; k++) buf[k] = marker[k % marker.Length];
    ArrayPool<byte>.Shared.Return(buf); // returned without clearing
}

var platform = new NodeEmbeddingPlatform(new NodeEmbeddingPlatformSettings
{
    Args = new[] { "node", "--disable-wasm-trap-handler" }
});

produces:

bad option: --disable-wasm-trap-handler_MARKER_1234567890_LEAKED_MARKER

Fix

Write the terminator explicitly instead of relying on the buffer being zeroed:

int written = Encoding.UTF8.GetBytes(
    src, strings[i].Length,
    (byte*)(stringBufferPtr + offset), byteLength - offset);
((byte*)stringBufferPtr)[offset + written] = 0;
offset += written + 1;

Clearing the rented buffer would also work, but writing the terminator is cheaper. The pointer array (ArrayPool<nint>.Shared.Rent) is not affected, since it is passed with an explicit count.

This is hard to diagnose because it passes in isolation and only surfaces under load or after warm-up. I can open a PR with the terminator fix and a test that poisons the pool if that would help.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in src/NodeApi/Runtime/Utf8StringArray.cs and reproduce the issue with the provided ArrayPool-poisoning example. Add a regression test covering pooled buffers on the .NET path, then run the relevant test suite. Done means embedding arguments remain null-terminated and no stale pool data appears in the native option string.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp, node.js
Domain
backend
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
82/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.