Possible `ExitEvent` leak when `BlockingCall()` returns `napi_closing`

Abierto Apto para principiantes
#938 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
2/5
Tiempo estimado
1-3 horas
Aptitud para principiantes
76/100
Tipo de issue
Error
Claridad
Bien especificado
Estado de actividad
Tranquilo
Stack tecnológico
cpp, node.js
Área
operating-systems

Línea de trabajo

Comienza en SetupExitCallback en src/unix/pty.cc y src/win/conpty.cc, siguiendo la propiedad en torno a BlockingCall y la rama napi_closing. Aplica la limpieza sugerida por el issue en ambas implementaciones y verifica después que ExitEvent se libera cuando el callback no se pone en cola y que, en caso contrario, permanece bajo la propiedad del callback.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

Possible ExitEvent leak when BlockingCall() returns napi_closing

I found a possible native heap leak in the pty exit watcher when the thread-safe function is closing.

Files: src/unix/pty.cc, src/win/conpty.cc

Function: SetupExitCallback

Relevant Unix code:

auto callback = [](Napi::Env env, Napi::Function cb, ExitEvent *exit_event) {
  cb.Call({Napi::Number::New(env, exit_event->exit_code),
           Napi::Number::New(env, exit_event->signal_code)});
  delete exit_event;
};

// ...

ExitEvent *exit_event = new ExitEvent;
// fill exit_event
auto status = tsfn.BlockingCall(exit_event, callback);
switch (status) {
  case napi_closing:
    break;

  case napi_queue_full:
    Napi::Error::Fatal("SetupExitCallback", "Queue was full");

  case napi_ok:
    if (tsfn.Release() != napi_ok) {
      Napi::Error::Fatal("SetupExitCallback", "ThreadSafeFunction.Release() failed");
    }
    break;
}

The Windows implementation has the same ownership pattern.

The ExitEvent is deleted only by the JS-thread callback. If BlockingCall()
returns napi_closing, the item was not queued and that callback will not run.
The case napi_closing: branch then drops the only pointer to exit_event.

This is a small teardown-race leak: one ExitEvent per pty whose exit races
Node-API environment shutdown.

Suggested fix: delete exit_event in the napi_closing branch, where ownership
was not transferred to the callback queue.

Lenguaje dominante
TypeScript
Estrellas
2k
Forks
337
Merge medio
21 h 58 min
PR fusionados (30 d)
3

Guía de contribución

Abrir la guía de contribución

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de microsoft/node-pty

Todos los issues de microsoft/node-pty

Issues similares

Más issues de TypeScript

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.