Skip to content

Fix updating lock leak in DashboardStateSync freezing URL/state sync #9701

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nishantmonu51 merged 3 commits into from
Jul 20, 2026
Merged

Fix updating lock leak in DashboardStateSync freezing URL/state sync #9701

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
038c148
Fix `updating` lock leak in `DashboardStateSync`
nishantmonu51 Jul 19, 2026
133cc16
Take the `updating` lock after guard clauses in `handleURLChange`
nishantmonu51 Jul 20, 2026
651c2c5
Release the `updating` lock before the redirect goto in `handleURLCha…
nishantmonu51 Jul 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
150 changes: 80 additions & 70 deletions web-common/src/features/dashboards/state-managers/loaders/DashboardStateSync.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,6 @@ export class DashboardStateSync {
// Since we call this in afterNavigation, there could be a scenario where navigation completes but data for init isnt loaded yet.
// Init already incorporates the url into the state so we can skip this processing.
if (this.updating || !this.initialized) return;
this.updating = true;

const { data: validSpecData } = get(this.dataLoader.validSpecQuery);
const metricsViewSpec = validSpecData?.metricsView ?? {};
Expand All @@ -236,56 +235,65 @@ export class DashboardStateSync {

const pageState = get(page);

if (metricsViewSpec.timeDimension && !import.meta.env.VITEST) {
// Resolve start/end by making a network call.
[
partialExplore.selectedTimeRange,
// partialExplore.selectedComparisonTimeRange,
] = await resolveTimeRanges(
this.client,
exploreSpec,
// Take the lock only once the guards have passed;
// the finally ensures a throw below cannot leave it stuck.
this.updating = true;
let redirectUrl: URL;
try {
if (metricsViewSpec.timeDimension && !import.meta.env.VITEST) {
// Resolve start/end by making a network call.
[
partialExplore.selectedTimeRange,
// partialExplore.selectedComparisonTimeRange,
],
partialExplore.selectedTimezone,
undefined,
partialExplore.selectedTimeDimension,
] = await resolveTimeRanges(
this.client,
exploreSpec,
[
partialExplore.selectedTimeRange,
// partialExplore.selectedComparisonTimeRange,
],
partialExplore.selectedTimezone,
undefined,
partialExplore.selectedTimeDimension,
);
}

// Merge the partial state from url into the store
metricsExplorerStore.mergePartialExplorerEntity(
this.exploreName,
partialExplore,
);
}

// Merge the partial state from url into the store
metricsExplorerStore.mergePartialExplorerEntity(
this.exploreName,
partialExplore,
);
// Get time controls state after explore state is updated.
const timeControlsState = get(this.timeControlStore);
// Get the updated URL, this could be different from the page url if we added extra state.
// The extra state could come from session storage, home bookmark or yaml defaults
const redirectUrl = this.getUrlForExploreState(partialExplore);

// Get the full updated state and save to session storage
const updatedExploreState =
get(metricsExplorerStore).entities[this.exploreName];
updateExploreSessionStore(
this.exploreName,
this.extraPrefix,
updatedExploreState,
exploreSpec,
metricsViewSpec,
timeControlsState,
);
if (!this.dataLoader.disableMostRecentDashboardState) {
// Update "most recent explore state" with updated state from url
saveMostRecentPartialExploreState(
// Get time controls state after explore state is updated.
const timeControlsState = get(this.timeControlStore);
// Get the updated URL, this could be different from the page url if we added extra state.
// The extra state could come from session storage, home bookmark or yaml defaults
redirectUrl = this.getUrlForExploreState(partialExplore);

// Get the full updated state and save to session storage
const updatedExploreState =
get(metricsExplorerStore).entities[this.exploreName];
updateExploreSessionStore(
this.exploreName,
this.extraPrefix,
updatedExploreState,
exploreSpec,
metricsViewSpec,
timeControlsState,
);
if (!this.dataLoader.disableMostRecentDashboardState) {
// Update "most recent explore state" with updated state from url
saveMostRecentPartialExploreState(
this.exploreName,
this.extraPrefix,
updatedExploreState,
);
}
} finally {
// Release before the goto below: state changes made while the navigation is in flight
// must still be picked up by gotoNewState.
this.updating = false;
}

this.updating = false;
// If the url doesn't need to be changed further then we can skip the goto
if (redirectUrl.search === pageState.url.search) {
return;
Expand All @@ -311,44 +319,46 @@ export class DashboardStateSync {
if (this.updating) return;
this.updating = true;

const { data: validSpecData } = get(this.dataLoader.validSpecQuery);
const exploreSpec = validSpecData?.explore ?? {};
const metricsViewSpec = validSpecData?.metricsView ?? {};
const timeControlsState = get(this.timeControlStore);
try {
const { data: validSpecData } = get(this.dataLoader.validSpecQuery);
const exploreSpec = validSpecData?.explore ?? {};
const metricsViewSpec = validSpecData?.metricsView ?? {};
const timeControlsState = get(this.timeControlStore);

const pageState = get(page);
const pageState = get(page);

// Get the new url params for the updated state
const newUrl = this.getUrlForExploreState(exploreState);
// Get the new url params for the updated state
const newUrl = this.getUrlForExploreState(exploreState);

// Update the session storage with the new explore state.
updateExploreSessionStore(
this.exploreName,
this.extraPrefix,
exploreState,
exploreSpec,
metricsViewSpec,
timeControlsState,
);
if (!this.dataLoader.disableMostRecentDashboardState) {
// Update "most recent explore state" with updated state.
// Since we do not update the state per action we do it here as blanket update.
saveMostRecentPartialExploreState(
// Update the session storage with the new explore state.
updateExploreSessionStore(
this.exploreName,
this.extraPrefix,
exploreState,
exploreSpec,
metricsViewSpec,
timeControlsState,
);
}
if (!this.dataLoader.disableMostRecentDashboardState) {
// Update "most recent explore state" with updated state.
// Since we do not update the state per action we do it here as blanket update.
saveMostRecentPartialExploreState(
this.exploreName,
this.extraPrefix,
exploreState,
);
}

// If the state didnt result in a new url then skip goto.
// This avoids adding redundant urls to the history.
if (newUrl.search === pageState.url.search) {
return;
}

// If the state didnt result in a new url then skip goto.
// This avoids adding redundant urls to the history.
if (newUrl.search === pageState.url.search) {
// dashboard changed so we should update the url
await goto(newUrl);
} finally {
this.updating = false;
return;
}

// dashboard changed so we should update the url
await goto(newUrl);
this.updating = false;
}
}
Loading