node:zlib: block worker shutdown on in-flight async compression (UAF) - #35155
Closed
robobun wants to merge 8 commits into
Closed
node:zlib: block worker shutdown on in-flight async compression (UAF)#35155robobun wants to merge 8 commits into
robobun wants to merge 8 commits into
Claude / Claude Code Review
completed
Jul 22, 2026 in 15m 30s
Code review found 1 important issue
Found 3 candidates, confirmed 3. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 1 |
| 🟡 Nit | 2 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | test/js/node/zlib/zlib-worker-terminate.test.ts:17-27 |
Test omits zstd lane; NativeZstd tag not covered |
| 🟡 Nit | test/js/node/zlib/zlib-worker-terminate.test.ts:58 |
Explicit per-test timeout violates test/CLAUDE.md; shrink workload instead |
Annotations
Check warning on line 27 in test/js/node/zlib/zlib-worker-terminate.test.ts
claude / Claude Code Review
Test omits zstd lane; NativeZstd tag not covered
The worker script exercises gzip/deflate (both → `NativeZlib`) and brotliCompress (→ `NativeBrotli`), but omits `zstdCompress` — so the `NativeZstd` task tag never reaches the barrier/drain, even though the PR description names zstd as an affected entry point and the fix is monomorphized for all three tags. Consider adding one more lane, e.g. `const zs = promisify(zlib.zstdCompress); lanes(1, () => zs(big.subarray(0, 4 << 20)));`, per REVIEW.md's "cover the variant matrix, not just the repro".
Check warning on line 58 in test/js/node/zlib/zlib-worker-terminate.test.ts
claude / Claude Code Review
Explicit per-test timeout violates test/CLAUDE.md; shrink workload instead
The explicit `30_000` per-test timeout violates test/CLAUDE.md ("**CRITICAL**: Do not set a timeout on tests") and REVIEW.md ("Don't raise per-test timeouts to make a slow test pass; shrink the workload"). The ~11s ASAN runtime comes from 4 rounds × 5 lanes over 4–16 MiB buffers; consider shrinking the buffers (e.g. 1–4 MiB — the lanes are already in flight when `"up"` arrives, so smaller chunks still land `terminate()` mid-`do_work()`) and/or trimming ROUNDS so it fits the default, then drop th
Loading