Skip to content

test(worker-threads): reap the spawned child after a timeout - #386

Merged
szegedi merged 1 commit into
mainfrom
szegedi/reap-worker-test-child
Aug 7, 2026
Merged

test(worker-threads): reap the spawned child after a timeout#386
szegedi merged 1 commit into
mainfrom
szegedi/reap-worker-test-child

Conversation

@szegedi

@szegedi szegedi commented Aug 7, 2026

Copy link
Copy Markdown

Symptom

win32-test-22 on #385 printed its mocha epilogue inside the first minute, then sat in_progress on Run ./.prebuildify/test for half an hour:

✔ should not crash when worker is terminated (19288ms)
91 passing (1m)
1 failing
1) Worker Threads should work:
   Error: Timeout of 20000ms exceeded.

Tests done in a minute, job still running 30 minutes later.

Screenshot 2026-08-07 at 09 44 00

Cause

A mocha timeout rejects the test but does not kill the process the test spawned, and npm test is nyc mocha … with no --exit, so mocha waits for the event loop to drain before exiting. A live child keeps its process handle on that loop, so a child that outlives its test holds the entire run open.

When the child is merely slow this is invisible — mocha waits the extra couple of seconds and exits, which is why it never shows up on Linux (worker.js finishes in ~4.5s there). When the child is wedged, the run never ends and the job burns a runner until the CI job limit instead of failing in ~20s.

Fix

promisify(execFile) exposes the ChildProcess on the returned promise, so capture it and kill it from afterEach — mocha still runs hooks after a timeout. Measured locally with the child replaced by a 10-minute sleep and the test timeout forced low:

total
with the reap 1.19s
without the reap never exits (killed by a 30s watchdog)

Chose this over adding --exit to the mocha invocation: --exit would paper over any handle leak, and this suite exists partly to catch worker threads that fail to exit.

Scope

This bounds the damage only. It does not address why that child wedges on win32, which is the actual bug and a separate investigation. It is worth noting that Windows takes the plain V8 CpuProfiler path (no SIGPROF, and worker.js sets withContexts/useCPED to false off-platform), and this codebase already carries detectV8Bug / v8ProfilerStuckEventLoopDetected machinery for V8 sampler stalls.

Also unaddressed here: should work has a 20s budget while its heavier in-process sibling gets 30s, and Windows runs these ~10x slower than Linux. That inconsistency is worth revisiting, but raising the timeout would not have fixed this run — the parent waits on the child regardless of the test timeout.

A mocha timeout rejects the test but does not kill the process the test
spawned, and `npm test` runs mocha without --exit, so mocha waits for the
event loop to drain before exiting. A live child keeps its process handle
on that loop, so a child outliving its test holds the whole run open. When
the child is merely slow this is invisible — mocha waits the extra couple
of seconds and exits. When the child is wedged, the run never ends and the
CI job burns a runner until the job limit instead of failing in seconds.

Seen on win32-test-22: the suite printed its epilogue inside the first
minute, then the job sat in_progress for half an hour on
`Run ./.prebuildify/test` after `should work` timed out.

Capture the ChildProcess (promisify(execFile) exposes it on the returned
promise) and kill it from afterEach, which mocha still runs after a
timeout. Measured with the child replaced by a 10-minute sleep and the
test timeout forced low:

    with the reap:     1.19s total
    without the reap:  never exits (killed by a 30s watchdog)

This only bounds the damage; it does not address why that child wedges on
win32 in the first place, which is a separate investigation. Reaping is
preferred over adding --exit to the mocha invocation: --exit would paper
over any handle leak, and this suite exists partly to catch worker threads
that fail to exit.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown

Overall package size

Self size: 2.45 MB
Deduped: 3.15 MB
No deduping: 3.15 MB

Dependency sizes | name | version | self size | total size | |------|---------|-----------|------------| | pprof-format | 2.3.0 | 503.97 kB | 503.97 kB | | source-map | 0.8.0 | 185.66 kB | 185.66 kB | | node-gyp-build | 4.8.4 | 13.86 kB | 13.86 kB |

🤖 This report was automatically generated by heaviest-objects-in-the-universe

@datadog-datadog-prod-us1

This comment has been minimized.

@szegedi szegedi added the semver-patch Bug or security fixes, mainly label Aug 7, 2026
@szegedi
szegedi enabled auto-merge (squash) August 7, 2026 07:56
@szegedi
szegedi merged commit ee8559d into main Aug 7, 2026
126 of 129 checks passed
@szegedi
szegedi deleted the szegedi/reap-worker-test-child branch August 7, 2026 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver-patch Bug or security fixes, mainly

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants