WebContent: Flush pending WebDriver messages before the process exits - #11028
Merged
AtkinsSJ merged 2 commits intoAug 7, 2026
Merged
Conversation
Problem: A sync IPC wait could fail with “Failed to receive message_id: N” even though the peer had sent the response, when the peer replied and then exited so the response and socket EOF were read in the same drain. Cause: wait_for_specific_endpoint_message_impl() scans the unprocessed messages, then blocks. On EOF it breaks out of the loop and reported failure without rescanning — but the drain that reported the EOF has just appended the awaited response to the queue, where it sits unread. Fix: After the loop breaks with the peer-disconnected flag set, scan the unprocessed messages once more and return the awaited response if it’s there. The drain scheduled its deferred handle_messages before its deferred shutdown, so any other messages from that final batch are still dispatched before the connection close is acted on.
Problem: TestWebDriverSessionHistory timeouts on the x86_64 Release GNU CI job in the cross-process no-content navigation-restore subtest. The harness waits 30s for the restored /a document — which never loads. https://github.com/LadybirdBrowser/ladybird/actions/runs/30988508142 https://github.com/LadybirdBrowser/ladybird/actions/runs/30999552155 https://github.com/LadybirdBrowser/ladybird/actions/runs/31042505962 https://github.com/LadybirdBrowser/ladybird/actions/runs/31082213133 Cause: When a cross-site navigation replaces a WebContent process, the UI tells the outgoing process to notify WebDriver of the window replace- ment, then immediately tells it to close. Both are async IPC messages: the notify only enqueues on the send queue for the IO thread to write, while close_server() exits the process via terminate_immediately(). Under load, the process exits before the IO thread flushes, so WebDriver never receives the replacement notice. It sees only the socket EOF, treats the window as closed, and tears the session down — SIGTERM-ing the UI mid-restore, so the restored /a document never loads. Fix: Before WebContent exits in close_server(), flush each page’s WebDriver transport with close_after_sending_all_pending_messages(), so the pending-replacement notice reaches WebDriver ahead of the EOF. The flush runs in close_server() rather than die() because die() can run during construction, before the page host is initialized.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe change preserves responses delivered during IPC peer disconnect handling. It also defers WebDriver connection closure until pending messages are sent during WebContent server shutdown. ChangesConnection shutdown handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ConnectionFromClient
participant PageHost
participant PageClient
participant WebDriverTransport
ConnectionFromClient->>PageHost: close WebDriver connections after pending messages
PageHost->>PageClient: request deferred closure
PageClient->>WebDriverTransport: close after flushing pending messages
ConnectionFromClient->>ConnectionFromClient: shut down IPC
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
AtkinsSJ
approved these changes
Aug 7, 2026
AtkinsSJ
left a comment
AtkinsSJ
left a comment
Member
There was a problem hiding this comment.
Thanks for tracking this down!
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
2 participants
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.
Problem:
TestWebDriverSessionHistorytimeouts on the x86_64 Release GNU CI job in the cross-process no-content navigation-restore subtest. The harness waits 30s for the restored/adocument — which never loads.https://github.com/LadybirdBrowser/ladybird/actions/runs/30988508142
https://github.com/LadybirdBrowser/ladybird/actions/runs/30999552155
https://github.com/LadybirdBrowser/ladybird/actions/runs/31042505962
https://github.com/LadybirdBrowser/ladybird/actions/runs/31082213133
Cause: When a cross-site navigation replaces a WebContent process, the UI tells the outgoing process to notify WebDriver of the window replacement, then immediately tells it to close. Both are async IPC messages: the notify only enqueues on the send queue for the IO thread to write, while
close_server()exits the process viaterminate_immediately(). Under load, the process exits before the IO thread flushes, so WebDriver never receives the replacement notice. It sees only the socket EOF, treats the window as closed, and tears the session down — SIGTERM-ing the UI mid-restore, so the restored/adocument never loads.Fix: Before WebContent exits in
close_server(), flush each page’s WebDriver transport withclose_after_sending_all_pending_messages(), so the pending-replacement notice reaches WebDriver ahead of the EOF. The flush runs inclose_server()rather thandie()becausedie()can run during construction, before the page host is initialized.The first commit in the branch here is a separate-but-related ordering bug found incidentally on the way: A sync IPC wait could report
Failed to receive message_id: Neven though the peer had sent the response — when the peer replied and then exited so that the response and the socket EOF were read in the same drain.wait_for_specific_endpoint_message_impl()broke out of its loop on EOF without rescanning the unprocessed messages the drain had just appended. It now rescans once — before reporting failure.