Repository navigation
Fix web leader-election deadlock with Web Locks (SequentialQueue unable to flush) - #104990
Conversation
… queue The leader used to be whichever client ID was last in ACTIVE_CLIENTS. A tab that closed without cleaning up could leave its ID last, and the live tab then never flushed the SequentialQueue. The leader is now the tab holding the activeClientLeader Web Lock, which the browser releases when the tab goes away. Each init() steals the lock, so the newest or focused tab still leads. While tabs from an older build are open, the lock holder steps aside when an older build's ID is last in ACTIVE_CLIENTS, so the two builds never both send. Browsers without Web Locks keep the old behavior.
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@thesahindia Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb4e3d24ec
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
The Web Lock is granted asynchronously, so the flush that runs when a tab gets focus can give up before the tab leads, and a waiting tab that takes over after the leader closes never flushed at all. ActiveClientManager now calls onBecomeLeader callbacks each time the tab becomes the leader, and the Network flushes the SequentialQueue from there.
|
No product review needed. |
|
the reassure tests seems flaky as I notice other pr's have this fail too |
|
Hi @thesahindia is there an update about it ? |
Reviewer Checklist
Screenshots/VideosAndroid: HybridApp
Android: mWeb ChromeScreen.Recording.2026-11-01.at.3.05.04.AM.moviOS: HybridApp
iOS: mWeb SafariScreen.Recording.2026-11-01.at.2.58.35.AM.movMacOS: Chrome / SafariScreen.Recording.2026-10-29.at.2.25.32.AM.movScreen.Recording.2026-10-29.at.2.18.34.AM.movScreen.Recording.2026-10-29.at.2.15.55.AM.mov |
|
@elirangoshen, please add these steps as well -
|
Is there anything specific that needs to be done to get "Another login page is open" ? Screen.Recording.2026-10-29.at.2.37.11.AM.mov |
|
Note: @elirangoshen is OOO today and will continue to work on this one tomorrow! |
Hi, The message only appears in a tab that's visible but not the leader. Switching to a tab makes it the leader, so in a single window every tab you switch to shows the form (same as on I updated the Tests and QA steps to make this clearer. |
done |
mountiny
left a comment
mountiny
left a comment
There was a problem hiding this comment.
Thanks! Looks good to me, i will move this ahead to land the fix sooner than later
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.5.28-4 🚀
|
|
No help site update needed. This PR fixes an internal bug in how browser tabs pick which one sends queued requests, so messages no longer get stuck. It adds no user-facing feature, and no help article describes this behavior. |
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.5.30-0 🚀
|
|
🚀 Deployed to production by https://github.com/blimpich in version: 9.5.30-0 🚀
Bundle Size Analysis (Sentry): |
Explanation of Change
On web and PWA, the app sometimes stops sending anything. Messages stay greyed out, the Inbox loading bar spins forever, and the logs show
[SequentialQueue] Unable to flush. Client is not the leader.on repeat.Why it happens. Only one tab (the "leader") sends the shared request queue. Today the leader is whichever client ID is last in the
ACTIVE_CLIENTSlist in Onyx. A tab removes its ID only inbeforeunload, which doesn't run when a tab crashes or the browser kills it in the background. The dead tab's ID can stay last, and then the tab that is still open never sends anything.What this PR does. The leader is now the tab holding the
activeClientLeaderWeb Lock:init()(page load or tab focus) takes the lock, so the newest or focused tab still leads, same as before. Other tabs wait for the lock.ACTIVE_CLIENTS. A new-build ID starts withwebLock_. While an old-build ID is last in the list, the lock holder steps aside, and it takes leadership back on its nextinit(). This way only one tab leads while old and new builds are open together.ACTIVE_CLIENTS, soSignInPagekeeps showing "Another login page is open" in the other tabs.contributingGuides/SEQUENTIAL_QUEUE.mdis updated to match.Fixed Issues
$ #94963
PROPOSAL: N/A — internal bug fix.
Automated Tests
New
tests/unit/ActiveClientManagerTest.tsruns several simulated tabs against a shared fake lock manager. It covers:ACTIVE_CLIENTSinit()at once give exactly one leaderTests
Web (Chrome), against staging:
AddComment.RequestNewValidateCodeand only one new email arrives.Offline tests
AddCommentacross both tabs.QA Steps
These steps can be done on a phone. Use mWeb Safari (iOS) and mWeb Chrome (Android), then repeat once on desktop Chrome / Safari.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari