Skip to content

test: wait for the parent's ack before utility children exit - #55082

Merged
jkleinsc merged 1 commit into
mainfrom
claude/utility-process-flush-before-exit
Oct 20, 2026
Merged

test: wait for the parent's ack before utility children exit#55082
jkleinsc merged 1 commit into
mainfrom
claude/utility-process-flush-before-exit

Conversation

@claude

@claude claude Bot commented Oct 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description of Change

As @deepak1556 pointed out, process.exit() does not flush asynchronous postMessage() writes, so a utility child that posts right before exiting can lose its message. Tests that need the message delivered now wait for the parent's ack before the child exits, restoring the handshake that #54924 removed in the api-net fixture and helper and adding it to the env-app and js2c-code-cache fixtures. The #54924 test no longer posts right before exiting: the child posts its parentPort messages, then posts on a transferred MessagePortMain and exits only when the parent replies there, which works as a barrier because parentPort and the port share the same ordered Mojo channel.

🤖 Generated with Claude Code

https://claude.ai/code/session_01LPMvJBQiyP6b7QjatACmnS

Checklist

Release Notes

Notes: none

@claude
claude Bot requested a review from jkleinsc October 17, 2026 14:31
@claude claude Bot added semver/patch backwards-compatible bug fixes 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 17, 2026
@claude
claude Bot marked this pull request as ready for review October 17, 2026 15:35
@deepak1556
deepak1556 self-requested a review October 19, 2026 11:26
deepak1556
deepak1556 requested changes Oct 19, 2026
View reviewed changes

@deepak1556 deepak1556 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should add a sync IPC round trip to the process.exit() path here. In Node.js, the api is explicitly documented to terminate the process immediately even when async I/O is still pending, and parentPort.postMessage() is an async IPC write.

The distinction from #54924 is that DispatchPendingMessages() drains messages that have already arrived on the browser's connector before emitting 'exit', whereas this PR tries to guarantee that in-flight child writes are flushed before process termination.

If a caller or test needs guaranteed delivery before calling process.exit(), it should wait for an acknowledgement from the parent (or exit after the parent handles the message) rather than calling postMessage() followed immediately by process.exit().

@claude

claude Bot commented Oct 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Thanks, that makes sense. DispatchPendingMessages() only covers messages that have already reached the browser, and nothing makes process.exit() wait for writes still queued in the child, so I'll drop the sync flush.

The catch is that the tests now need what was taken out in #54924. @ckerr and @dsanders11 asked for the ack handshake in api-net-spec.js to be removed once the drain landed, and that removal is why the Windows credentials: 'omit' cookies test still sometimes fails with "exited before posting a result". There's a test-only alternative on claude/ack-before-exit. It restores the ack handshake in the api-net fixture and helper, keeping #54901's error logging so late async errors can't change the exit code. It adds the same ack to the env-app and js2c-code-cache fixtures, which also post and then exit. It also reworks #54924's "delivers messages the child posted right before exiting" test, which asserts exactly the case that isn't guaranteed. An ack would stop that test from exercising the drain, so the choice there is a short delay before exit (the branch uses 250ms, which is still timing-based) or deleting it. I'd lean towards deleting it.

If this direction works, I'll replace this PR's change with that branch, retitled as test: with semver/none.

Generated by Claude Code

@deepak1556

deepak1556 commented Oct 19, 2026

Copy link
Copy Markdown
Member

Yes, let's bring back the ack handshake in api-net-spec.js, env-app, and js2c-code-cache. For post-message-then-exit.js, avoid artificial delays and instead use a transferred MessagePortMain as a barrier. Since parentPort and the MessagePortMain share the same ordered Mojo channel.

@claude
test: wait for the parent's ack before utility children exit
6c21301 
process.exit() does not wait for pending postMessage() writes, so a
utility child that exits right after posting can lose its message.
Restore the acknowledgement handshake that #54924 removed from the
api-net fixture and helper, and add it to the env-app and
js2c-code-cache fixtures, so those children only exit once the parent
has acknowledged their message.

The #54924 test no longer posts right before exiting. The child posts
its parentPort messages, then posts on a transferred MessagePortMain and
exits only when the parent replies there. parentPort and the port share
the same ordered Mojo channel, so all of the parentPort messages have
reached the parent when the port message does.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LPMvJBQiyP6b7QjatACmnS
@claude
claude Bot force-pushed the claude/utility-process-flush-before-exit branch from 6895c95 to 6c21301 Compare October 19, 2026 16:04
@claude claude Bot changed the title fix: flush pending utilityProcess messages before the child exits on Windows test: wait for the parent's ack before utility children exit Oct 19, 2026
@claude claude Bot added semver/none and removed semver/patch backwards-compatible bug fixes labels Oct 19, 2026
@claude
claude Bot requested a review from deepak1556 October 19, 2026 16:04
@claude

claude Bot commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

Updated in 6c21301. The ack handshake is back in api-net-spec.js, env-app and js2c-code-cache. The #54924 test now uses a transferred MessagePortMain as the barrier, with no timers. On v46.0.0-nightly.20261016 it passed 10/10, as did the api-net utility cookie tests.

One thing to flag: with the barrier, the test no longer exercises the #54924 drain. When the browser's Connector reads the first parentPort message, it queues dispatch tasks for everything else already in the pipe. So by the time the port signal reaches JS, all 20 messages are queued ahead of the child's exit, and DispatchPendingMessages() never finds anything. The new test also passes 10/10 on v46.0.0-nightly.20261014, which predates #54924, while the old version fails 3/3 there. The drain can only be reached if the child exits without waiting for a reply, which is the pattern you ruled out. Unless you'd prefer otherwise, I'll leave the drain without a dedicated spec and rename this test to say what it now checks: ordering between parentPort and a transferred port.

Generated by Claude Code

@jkleinsc
jkleinsc merged commit 23c730b into main Oct 20, 2026
86 checks passed
@release-clerk

release-clerk Bot commented Oct 20, 2026

Copy link
Copy Markdown

No Release Notes

@jkleinsc
jkleinsc deleted the claude/utility-process-flush-before-exit branch October 20, 2026 00:11
@trop trop Bot mentioned this pull request Oct 20, 2026
Merged
@trop

trop Bot commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

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

This was referenced Oct 20, 2026
Merged
Merged
@trop trop Bot removed the target/43-x-y PR should also be added to the "43-x-y" branch. label Oct 20, 2026
@trop trop Bot mentioned this pull request Oct 20, 2026
Merged
@trop

trop Bot commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

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

@trop

trop Bot commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

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

@trop

trop Bot commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

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

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

Reviewers

@jkleinsc jkleinsc jkleinsc approved these changes
@deepak1556 deepak1556 deepak1556 approved these changes

Assignees

No one assigned

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

46-x-y
Status: No status

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@deepak1556 @jkleinsc @claude