Repository navigation
fix(frame): make check and subscribe one step in the navigation waits - #3388
Closed
Yury Semikhatsky (yury-s) wants to merge 2 commits into
Closed
Yury Semikhatsky (yury-s) wants to merge 2 commits into
Yury Semikhatsky (yury-s) wants to merge 2 commits into
Conversation
Load states are one TaskCompletionSource per WaitUntilState that the pump completes, so WaitForLoadStateAsync and WaitForNavigationAsync await the current slot instead of checking a list and then subscribing. WaitForURLAsync matches Url and subscribes under the dispatch lock that OnNavigated takes to update Url. Fixes: #3364
Yury Semikhatsky (yury-s)
force-pushed
the
dispatch-lock
branch
from
October 22, 2026 23:23
365e873 to
84d33c0
Compare
Yury Semikhatsky (yury-s)
force-pushed
the
fix-3364-one-step
branch
from
October 22, 2026 23:29
711cfe4 to
ae8931b
Compare
The matcher can be a user predicate, and a predicate that blocks on a Playwright call deadlocked while the lock was held: the pump waits for the lock at the next frame, page or worker message, so the reply never arrives. Read Url and a navigation count under the lock, match outside it, and subscribe under the lock only if no navigation landed since, otherwise match again.
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.
Summary
TaskCompletionSourceperWaitUntilState: a completed slot means the state is present, a pending one that it is not, andWaitForLoadStateAsyncandWaitForNavigationAsyncawait the current slot. There is no check to interleave with a subscription, and the torn read of theList<WaitUntilState>is gone.WaitForURLAsynckeeps the JS shape. It readsUrland a navigation count underConnection.DispatchLock, runs the matcher outside the lock because it can be a user predicate, and subscribes toNavigatedunder the lock only if the count has not moved, otherwise it matches again.OnNavigatedupdatesUrl, bumps the count and snapshots the handlers under the same lock.frameAttachedmessage; it passes onmainand would deadlock if the matcher ran under the lock.mainin 2 of 3 fresh test hosts, passes 3 of 3 here.Stacked on #3387, which adds the lock. Replaces #3384. Load-state waits now resume on the thread pool rather than inline on the reader thread, which three tests in #3386 currently pin.
Fixes #3364