microsoft / microsoft/node-api-dotnet

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

Offen Anfängerfreundlich
#481 0 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen

Dieses Issue hat noch niemand übernommen.

Vorherrschende Sprache
C#
Sterne
783
Forks
80
PR-Merge-Kennzahlen
Keine gemergten PRs in 30 T.

Beschreibung

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.

Beitragsleitfaden

Beitragsleitfaden öffnen

Erste Schritte

  1. Lies das ganze Issue und danach den Beitragsleitfaden des Projekts.
  2. Schreib ins Issue, dass du es übernimmst — das erspart doppelte Arbeit.
  3. Forke das Repository und arbeite in einem Branch.
  4. Öffne einen Pull Request, der die Issue-Nummer nennt.

Rechercherichtung

Beginnen Sie in src/NodeApi/Runtime/Utf8StringArray.cs und reproduzieren Sie das Problem mit dem bereitgestellten ArrayPool-Vergiftungsbeispiel. Fügen Sie einen Regressionstest hinzu, der gepoolte Puffer auf dem .NET-Pfad abdeckt, und führen Sie anschließend die relevante Testsuite aus. Als abgeschlossen gilt die Änderung, wenn die eingebetteten Argumente weiterhin nullterminiert sind und keine veralteten Pool-Daten in der nativen Optionszeichenfolge erscheinen.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Bewertung

Tech-Stack
csharp, node.js
Bereich
backend
Issue-Typ
Bug
Schwierigkeit
2/5
Geschätzter Aufwand
1-3 Stunden
Aktivitätsstatus
Ruhig
Klarheit
Klar beschrieben
Anfängerfreundlichkeit
82/100

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.