AsyncSRT.dispose()/worker.terminate() causes native heap corruption (reproduces on v1.5.5 and v1.5.6)

Abierto
  • ¿Algo no está bien?
#91 1 comentario 0 reacciones 0 asignados Ver en GitHub

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
4/5
Tiempo estimado
3-5 días
Aptitud para principiantes
48/100
Tipo de issue
Error
Claridad
Bastante claro
Estado de actividad
Tranquilo
Stack tecnológico
javascript, node.js
Área
backend, networking

Línea de trabajo

Empieza reproduciendo el escenario de close-then-dispose en spec/async_srt_await_spec.js y lee la implementación de AsyncSRT en src/async.js. Compara las especificaciones async existentes; después, haz que la liberación espere a que se complete el desmontaje del socket nativo antes del apagado del worker y añade cobertura para close plus dispose; se considera terminado cuando las especificaciones pasan sin corrupción del heap ni fallos durante el desmontaje.

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

Descripción

bug ready-for-maintainer

Summary

AsyncSRT.dispose() (which calls Worker.terminate() under the hood — see src/async.js) causes native heap corruption when called on a worker that has active or recently-closed SRT sockets. This reproduces on both v1.5.5 and v1.5.6 of the bundled libSRT — it is unrelated to the version-bump work in #89/#90.

This code path currently has no test coverage: dispose()/worker.terminate() is never called in spec/async_srt_await_spec.js (it's commented out) and is absent from every other spec file.

Reproduction

In a spec exercising AsyncSRT: create socket(s), close them via the async API, then call dispose() on the AsyncSRT instance (which calls Worker.terminate()) before the process exits.

Observed on Linux (node:20-bookworm, arm64, matching CI's toolchain):

free(): invalid pointer

and, on other runs of the same scenario:

free(): invalid next size (fast)

and one occurrence of a V8 fatal error during worker teardown:

Check failed: node->IsInUse().

All are consistent with a race between Worker.terminate() tearing down the worker thread/its Realm and the native addon's own in-flight close()/socket-teardown state still running inside that same worker thread — i.e. terminate() doesn't wait for the native binding's cleanup to finish before the thread (and its heap) is torn down.

Why this matters

Heap corruption (free(): invalid pointer / invalid next size) is a use-after-free/double-free class bug — exploitable in principle, not just a crash. Any application using AsyncSRT.dispose() for graceful shutdown (a reasonable, expected use of a public API method) can hit this today, independent of which libSRT version is bundled.

Suggested fix direction

Worker.terminate() is inherently abrupt — it doesn't let in-flight work finish. A safer dispose() likely needs to:

  • signal the worker to finish any pending native close()/socket teardown and acknowledge completion (e.g. a message round-trip) before calling worker.terminate(), or
  • avoid terminate() entirely in favor of a graceful shutdown message the worker acts on and then exits on its own.

Suggested test coverage

Un-comment and fix the existing dispose() call in spec/async_srt_await_spec.js, and add explicit close+dispose coverage to the other async spec files, so this path is exercised by CI going forward.

Found incidentally while investigating #89/#90 (libSRT v1.5.6 CVE fix); filed separately since it's unrelated to that work and affects the current release.

Lenguaje dominante
JavaScript
Estrellas
99
Forks
35
Sin PR fusionados en 30 d

Preparar el entorno

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 Eyevinn/node-srt

Todos los issues de Eyevinn/node-srt

Issues similares

Más issues de JavaScript

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.