Skip to content

test: clean up the near-heap-limit snapshot test's child and temp dir on failure - #54940

Merged
ckerr merged 1 commit into
mainfrom
fix/utility-process-near-heap-limit-snapshot
Oct 15, 2026
Merged

ckerr merged 1 commit into
mainfrom
fix/utility-process-near-heap-limit-snapshot

Conversation

@ckerr

@ckerr ckerr commented Oct 14, 2026

Copy link
Copy Markdown
Member

Description of Change

utilityProcess module behavior supports generating snapshots via v8.setHeapSnapshotNearHeapLimit has been running close to its 60 s budget on upstream CI. Passing macos-x64 runs take up to 56 s, and main run 38585284171 needed a retry on linux-x64-asan (98.6 s) and macos-x64 (94.3 s). #54901 sped up the oom-grow.js fixture that made it slow.

This PR fixes what happens when an attempt still fails or times out:

  • Nothing killed the attempt's utility process, so it kept running during CI's retry.
  • The diagnostic directory was removed only when the test passed.

The test now registers both cleanups with defer() as soon as they are needed: deferKillUtilityProcess(child) right after fork(), and a forced fs.rm() of the directory right after mkdtemp(). Cleanup runs in reverse order, so the child is killed before its directory is removed.

Tested with a fixture that stalls below the heap limit and retry: 1:

  • Before this change, two stalled children were alive at once and two directories were left in the temp dir.
  • After it, at most one child was alive and no directories were left.

The automatic v8.setHeapSnapshotNearHeapLimit(1) path and the exactly-one, non-empty snapshot assertions are unchanged.

On 42-x-y and 43-x-y, the backport also needs defer added to the ./lib/spec-helpers.ts import; otherwise the test throws ReferenceError: defer is not defined.

🤖 Generated with Claude Code

Checklist

Release Notes

Notes: none

… on failure

The setHeapSnapshotNearHeapLimit spec removed its diagnostic directory
only when it passed, and nothing killed its utility process
if an attempt failed or timed out.
CI retries this test, and on upstream main it has been running close to
its 60 s budget: passing macos-x64 runs take up to 56 s,
and main run 38585284171 needed a retry on linux-x64-asan (98.6 s)
and on macos-x64 (94.3 s).
A timed-out attempt's child kept running during the retry,
and each failed attempt left its directory behind in the temp dir.

Register both cleanups with defer() as soon as they are needed:
deferKillUtilityProcess() for the child right after fork(),
and a forced fs.rm() for the directory right after mkdtemp().
defer() runs them in reverse order,
so the child is killed before its directory is removed.
The automatic snapshot path and the exactly-one, non-empty snapshot
assertions are unchanged.
The fixture that made the test slow is sped up in #54901.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ckerr ckerr added semver/none target/42-x-y PR should also be added to the "42-x-y" branch. target/43-x-y PR should also be added to the "43-x-y" branch. target/44-x-y PR should also be added to the "44-x-y" branch. target/45-x-y PR should also be added to the "45-x-y" branch. labels Oct 14, 2026
@ckerr
ckerr merged commit 9e30adc into main Oct 15, 2026
136 checks passed
@ckerr
ckerr deleted the fix/utility-process-near-heap-limit-snapshot branch October 15, 2026 02:32
@release-clerk

release-clerk Bot commented Oct 15, 2026

Copy link
Copy Markdown

No Release Notes

@trop

trop Bot commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

I have automatically backported this PR to "42-x-y", please check out #54977

@trop

trop Bot commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

I have automatically backported this PR to "43-x-y", please check out #54978

@trop

trop Bot commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

I have automatically backported this PR to "44-x-y", please check out #54979

@trop trop Bot removed the target/42-x-y PR should also be added to the "42-x-y" branch. label Oct 15, 2026
@trop

trop Bot commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

I have automatically backported this PR to "45-x-y", please check out #54980

@trop trop Bot added in-flight/44-x-y in-flight/45-x-y and removed target/43-x-y PR should also be added to the "43-x-y" branch. labels Oct 15, 2026
@trop trop Bot added merged/45-x-y PR was merged to the "45-x-y" branch. merged/44-x-y PR was merged to the "44-x-y" branch. merged/43-x-y PR was merged to the "43-x-y" branch. merged/42-x-y PR was merged to the "42-x-y" branch. and removed target/44-x-y PR should also be added to the "44-x-y" branch. target/45-x-y PR should also be added to the "45-x-y" branch. in-flight/45-x-y in-flight/44-x-y in-flight/43-x-y in-flight/42-x-y labels Oct 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

46-x-y merged/42-x-y PR was merged to the "42-x-y" branch. merged/43-x-y PR was merged to the "43-x-y" branch. merged/44-x-y PR was merged to the "44-x-y" branch. merged/45-x-y PR was merged to the "45-x-y" branch. semver/none

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants