Fix updating lock leak in DashboardStateSync freezing URL/state sync#9701
Merged
Conversation
Wrap handleURLChange and gotoNewState in try/finally so the updating lock is released on early returns and throws. Previously a bare return (unloaded default URL params or explore state) left the lock stuck, permanently freezing URL and session-storage sync until a page reload.
Collaborator
Author
|
@codex: review |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
AdityaHegde
requested changes
Jul 20, 2026
AdityaHegde
left a comment
Collaborator
There was a problem hiding this comment.
Wrapping in try-catch will just suppress the issue. How about moving the setting of updating after guard clauses?
Also where are you seeing issues around this? There might be another undelying issue needing fix.
AdityaHegde
approved these changes
Jul 20, 2026
…nge` Holding the lock through the awaited goto made gotoNewState drop any state change that landed while the navigation was in flight, so the URL silently missed those updates. Restore the original release point (before the goto) and keep the try/finally only around the state-merge section for the throw paths.
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.
handleURLChangesetthis.updating = trueand only reset it on the success path; the two guard clauses (!rillDefaultExploreURLParams,!partialExplore) returned without releasing the lock. Once leaked, every subsequenthandleURLChangeandgotoNewStateshort-circuited, so filter/time/pivot changes stopped updating the URL, session storage, and the most-recent-state store until a full page reload.try/finallyreleases it ifresolveTimeRangesthrows. The lock is released before the finalgoto(as before), since state changes made while the navigation is in flight must still be picked up bygotoNewState.gotoNewStategets the sametry/finallytreatment with its original release point (afterawait goto) unchanged.Checklist: