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

未关闭
#91 1 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看

还没有人认领这个 Issue。

评估

难度
4/5
预计耗时
3-5 天
新手友好度
48/100
Issue 类型
缺陷
描述清晰度
基本清楚
活跃度
冷清

调研方向

首先在 spec/async_srt_await_spec.js 中复现 close-then-dispose 场景,并阅读 src/async.js 中的 AsyncSRT 实现。对比现有的 async spec,然后让 dispose 在 worker shutdown 之前等待 native socket teardown,并为 close plus dispose 添加 coverage;当 spec 在没有 heap-corruption 或 teardown 失败的情况下通过时,即表示完成。

由索引模型根据 Issue 内容生成。

描述

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.

主要语言
JavaScript
星标
99
派生
35
PR 合并指标
30 天内没有已合并 PR

环境准备

  • 没有 Dockerfile 或 Docker Compose 文件
  • 没有 Pull Request 模板
  • 阅读贡献指南

从这里开始

  1. 先读完整个 Issue,再读项目的贡献指南。
  2. 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
  3. Fork 仓库,在一个分支上完成修改。
  4. 提交 Pull Request,并在描述里引用这个 Issue 编号。

Eyevinn/node-srt 的其他 Issue

查看 Eyevinn/node-srt 的全部 Issue

相似的 Issue

更多 JavaScript Issue

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。