ConPTY/TSFN exit callback aborts the process during environment teardown — fixable with NODE_API_SWALLOW_UNTHROWABLE_EXCEPTIONS (same root cause as #904)

Aperta Adatta ai principianti
#951 1 commento 0 reazioni 0 assegnatari Vedi su GitHub

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
2/5
Tempo stimato
1-3 ore
Idoneità per principianti
76/100
Tipo di issue
Bug
Chiarezza
Specificata chiaramente
Stato di attività
Tranquilla
Stack tecnologico
cpp, node.js

Direzione di ricerca

Inizia con binding.gyp e con except.gypi e il guard di napi-inl.h di node-addon-api, quindi esamina l’exit callback in src/win/conpty.cc:94. Conferma che la macro venga compilata in node-pty e valida una build Windows/ConPTY durante il teardown dell’ambiente; il lavoro è completato quando il callback non interrompe più il processo.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

Summary

A ThreadSafeFunction exit callback that fires while the Node environment is terminating aborts the process. On Windows this is 0xc0000409 / FAST_FAIL_FATAL_APP_EXIT; on macOS it is the SIGABRT in #904.

The fix is one line in binding.gyp: define NODE_API_SWALLOW_UNTHROWABLE_EXCEPTIONS. node-addon-api already contains the guard for exactly this situation, and node-pty does not compile it in.

I believe this is the same defect as #904 (macOS, Environment::RunCleanup) and plausibly the mechanism behind #938. I have Windows/ConPTY evidence for it below.

Evidence

Downstream: amirlehmam/wmux#150 — 13 crashes across 0.10 → 1.1.0, one signature, over 13 months. Reported and analysed by @Ray0483, who did the dump forensics; the version-independence is theirs, not mine.

Exception code:       0xc0000409          (STATUS_STACK_BUFFER_OVERRUN)
Additional parameter: 0x7                 (FAST_FAIL_FATAL_APP_EXIT -> abort())
Faulting module:      wmux.exe            (throw originates in conpty.node)

The C++ EH record decodes to a real throw, not an SEH fault (ExceptionInformation[0] == 0x19930520, NumberParameters=4), and ExceptionInformation[3] — the throw's image base — equals conpty.node's load address in every dump. Walking _ThrowInfo -> _CatchableTypeArray -> TypeDescriptor:

.?AVError@Napi@@            <- Napi::Error
.?AVObjectReference@Napi@@
.?AV?$Reference@VObject@Napi@@@Napi@@
.?AVexception@std@@

conpty.dll loaded and winpty absent in all of them, so every occurrence is the useConptyDll: true path.

A full-memory dump then gave the decisive fact — the thrown Napi::Error's napi_ref resolves to a live V8 heap object whose message is:

'An exception is pending'

That string is not JS-authored. It is napi_extended_error_info.error_message for status napi_pending_exception.

Why that string is a proof rather than a hint

Against node-addon-api 7.1.1, the version node-pty@1.1.0 resolves.

1. Error::New(napi_env) only produces that message on one branch (napi-inl.h:2822):

status = napi_is_exception_pending(env, &is_exception_pending);
if (is_exception_pending) {
  status = napi_get_and_clear_last_exception(env, &error);   // adopts the real JS error
} else {
  const char* error_message = last_error_info_copy.error_message ...;  // <- 'An exception is pending'
  ...
}

So observing it proves napi_is_exception_pending returned false, while the immediately preceding N-API call had failed with napi_pending_exception.

2. Only one state satisfies both. In Node's js_native_api_v8.cc, NAPI_PREAMBLE returns napi_pending_exception when !(env->last_exception.IsEmpty() && env->can_call_into_js()), whereas napi_is_exception_pending reports only !last_exception.IsEmpty(). A false from the second with a napi_pending_exception from the first therefore means can_call_into_js() == falsethe environment is stopping. (This step is from Node's source rather than from a header in my tree; everything else here I read directly.)

3. It is unhandleable by design, and node-addon-api says so (napi-inl.h:3039):

inline void Error::ThrowAsJavaScriptException() const {
#ifdef NODE_API_SWALLOW_UNTHROWABLE_EXCEPTIONS
    ...
    if (status == napi_pending_exception) {
      // The environment must be terminating as we checked earlier and there
      // was no pending exception. In this case continuing will result
      // in a fatal error and there is nothing the author has done incorrectly
      // in their code that is worth flagging through a fatal error
      return;                                    // <- the guard
    }
#else
    napi_status status = napi_throw(_env, Value());
#endif

#ifdef NAPI_CPP_EXCEPTIONS
    if (status != napi_ok) {
      throw Error::New(_env);                    // <- uncaught, from a frame with nothing above it
    }
#endif

4. node-pty does not define the macro. binding.gyp depends on node_addon_api_except, whose except.gypi defines NAPI_CPP_EXCEPTIONS and nothing else. NODE_API_SWALLOW_UNTHROWABLE_EXCEPTIONS appears nowhere in the package. So the #else compiles in, and the guard written for this exact scenario is absent from the shipped prebuilds.

The full sequence

src/win/conpty.cc:94:

auto callback = [](Napi::Env env, Napi::Function cb, ExitEvent *exit_event) {
  cb.Call({Napi::Number::New(env, exit_event->exit_code)});
  delete exit_event;
};
  1. A PTY child exits; the TSFN dispatcher runs this on the main thread.
  2. node-addon-api wraps it in WrapVoidCallback (napi-inl.h:95), which catches Napi::Error and calls ThrowAsJavaScriptException().
  3. The environment is terminating, so Napi::Number::New / cb.Call fail with napi_pending_exception and throw a Napi::Error carrying the synthesized 'An exception is pending'. <- throw #1
  4. WrapVoidCallback catches it and calls ThrowAsJavaScriptException().
  5. napi_throw cannot throw into a dying isolate. Without the swallow guard, throw Error::New(_env). <- throw #2, uncaught
  6. Nothing above that frame -> UnhandledExceptionFilter -> abort() -> __fastfail(7) -> 0xc0000409.

This also accounts for an observation that was previously unexplained: the faulting thread's stack carries exactly two live C++ throw records, at byte-identical offsets across dumps taken months and five releases apart. Steps 3 and 5.

Napi::Number::New being an argument matters — it is evaluated before cb.Call is entered, so this can begin before control ever reaches the JS callback. A downstream try/catch around the JS function cannot see it. We shipped one and it changed nothing, which is consistent.

Suggested fix

'target_defaults': {
  'defines': [ 'NODE_API_SWALLOW_UNTHROWABLE_EXCEPTIONS' ],

That turns step 5 into a return, which is what the macro exists for, and makes teardown a no-op instead of an abort. It changes nothing on any path where the environment is alive.

Belt and braces, and independently worthwhile: guard the callback body itself, so a failure during teardown does not proceed to cb.Call and does not leak the ExitEvent (which I think is #938):

auto callback = [](Napi::Env env, Napi::Function cb, ExitEvent *exit_event) {
  std::unique_ptr<ExitEvent> owned(exit_event);
  try {
    cb.Call({Napi::Number::New(env, owned->exit_code)});
  } catch (const Napi::Error&) {
    // Environment teardown. Nothing to report to, and nowhere to report it.
  }
};

Reproduction

Not deterministic — it is a race with environment teardown. Downstream it ran about one crash every 2–4 days under normal use, and both crashes we have context for happened within seconds of an OS-driven session end (one a Windows Update restart, 57 seconds later). #904 reproduces it reliably by having Playwright close Electron windows with live PTYs, which is the same race deliberately provoked.

Happy to help

@Ray0483 has nine dumps and the parser to read them, and has offered to answer targeted questions about specific structures without publishing the files (they carry the process environment block — please do not ask anyone for these casually). I can test a patched build on the Windows side.

I would also be glad to send the binding.gyp one-liner as a PR if you would take it.

Lingua principale
TypeScript
Stelle
2k
Fork
337
Merge medio
21h 58m
PR unite (30g)
3

Guida per i contributori

Apri la guida per i contributori

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di microsoft/node-pty

Tutte le issue di microsoft/node-pty

Issue simili

Altre issue su TypeScript

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.