Skip to content

test: deflake crash-spec "js-execute-iframe" afterEach by force-killing wedged children#52148

Merged
MarshallOfSound merged 5 commits into
mainfrom
claude/fix/deflake-crash-js-execute-iframe
Jun 26, 2026
Merged

test: deflake crash-spec "js-execute-iframe" afterEach by force-killing wedged children#52148
MarshallOfSound merged 5 commits into
mainfrom
claude/fix/deflake-crash-js-execute-iframe

Conversation

@claude

@claude claude Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Requested by Samuel Attard · Slack thread

Before / After

Before: On busy Linux CI runners the crash cases "js-execute-iframe" test intermittently fails in its after each hook with waitUntil timed out after 10000ms (~7.5% of runs), going red before Electron's test reranner masks it on retry.

After: The hook tears down reliably even when a crash fixture's child process is wedged, so it no longer times out.

How

The afterEach sent the default SIGTERM to each spawned child and then polled waitUntil(() => children.length === 0). The children array only shrinks when each child's own exit handler fires. A wedged crash fixture (the whole point of these cases) can ignore SIGTERM and never emit exit, so the count never reaches 0 and the 10s waitUntil times out in the hook even though the test body passed.

The hook now attaches the exit listener first, sends SIGKILL (which cannot be caught or ignored, so the process goes down promptly), and awaits each child's real exit — removing the dependency on the side-effecting array length. Children that have already exited are skipped (guarding against ESRCH on a dead pid).

Verification

Root cause confirmed from CI logs of runs 28199457464 and 28199339066 (the recorded signal is the afterEach waitUntil timeout following the fixture's own 30s test-body timeout — not the benign surface_manager GPU log noise, which appears equally in passing runs). Locally: 27/27 crash cases pass normally, and 15/15 iterations pass under sustained CPU load.

Closes #52140.

Notes: none

… wedged children

The afterEach sent the default SIGTERM then polled children.length===0, which only drops when each child emits exit. A wedged crash fixture child can ignore SIGTERM and never emit exit, so the 10s waitUntil timed out. Send SIGKILL and await each child's real exit instead.
@MarshallOfSound MarshallOfSound marked this pull request as ready for review June 26, 2026 03:32
@dsanders11 dsanders11 added target/41-x-y PR should also be added to the "41-x-y" branch. 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. labels Jun 26, 2026
claude Bot added 3 commits June 26, 2026 16:19
…er document-end emit

ExplicitMicrotasksScope flips policy to kExplicit and asserts scope depth 0 on entry, which fails when document-end runs re-entrantly (window.open synchronous about:blank/iframe commit). A plain v8::MicrotasksScope nests correctly and still satisfies the CallDepthScope DCHECK, matching the IPC emit path.
@MarshallOfSound MarshallOfSound merged commit a24d0a6 into main Jun 26, 2026
78 checks passed
@MarshallOfSound MarshallOfSound deleted the claude/fix/deflake-crash-js-execute-iframe branch June 26, 2026 19:19
@release-clerk

release-clerk Bot commented Jun 26, 2026

Copy link
Copy Markdown

No Release Notes

@trop

trop Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

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

@trop

trop Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

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

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

trop Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

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

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

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Flaky test: crash cases "js-execute-iframe" after-each hook times out (linux)

2 participants