Repository navigation
test: clean up the near-heap-limit snapshot test's child and temp dir on failure - #54940
Merged
Merged
Conversation
… 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>
deepak1556
approved these changes
Oct 15, 2026
|
No Release Notes |
This was referenced Oct 15, 2026
Contributor
|
I have automatically backported this PR to "42-x-y", please check out #54977 |
Contributor
|
I have automatically backported this PR to "43-x-y", please check out #54978 |
Contributor
|
I have automatically backported this PR to "44-x-y", please check out #54979 |
Contributor
|
I have automatically backported this PR to "45-x-y", please check out #54980 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of Change
utilityProcess module behavior supports generating snapshots via v8.setHeapSnapshotNearHeapLimithas 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 theoom-grow.jsfixture that made it slow.This PR fixes what happens when an attempt still fails or times out:
The test now registers both cleanups with
defer()as soon as they are needed:deferKillUtilityProcess(child)right afterfork(), and a forcedfs.rm()of the directory right aftermkdtemp(). 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: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
deferadded to the./lib/spec-helpers.tsimport; otherwise the test throwsReferenceError: defer is not defined.🤖 Generated with Claude Code
Checklist
Release Notes
Notes: none