ui: fix crash with flamegraph on remount of area select (#7109)
diff --git a/ui/src/components/flamegraph_panel.ts b/ui/src/components/flamegraph_panel.ts
index 03451b2..43e58a0 100644
--- a/ui/src/components/flamegraph_panel.ts
+++ b/ui/src/components/flamegraph_panel.ts
@@ -14,7 +14,11 @@
import type m from 'mithril';
import type {Trace} from '../public/trace';
-import {QueryFlamegraph, type QueryFlamegraphMetric} from './query_flamegraph';
+import {
+ QueryFlamegraph,
+ type QueryFlamegraphDependency,
+ type QueryFlamegraphMetric,
+} from './query_flamegraph';
import type {
FlamegraphAddableMetric,
FlamegraphState,
@@ -36,10 +40,10 @@
readonly addableMetrics?: ReadonlyArray<FlamegraphAddableMetric>;
readonly onAddMetric?: (metric: FlamegraphAddableMetric) => void;
- // Perfetto tables / indices the metric SQL depends on. The panel forwards
- // them to the inner `QueryFlamegraph`, which disposes them along with
- // itself on unmount or when the array reference changes.
- readonly dependencies?: ReadonlyArray<AsyncDisposable>;
+ // Shared Perfetto tables / indices the metric SQL depends on. The caller
+ // retains ownership; the panel keeps a clone alive until its inner
+ // `QueryFlamegraph` is disposed.
+ readonly dependencies?: ReadonlyArray<QueryFlamegraphDependency>;
}
// Mithril wrapper around `QueryFlamegraph` that owns the inner instance's
diff --git a/ui/src/components/query_flamegraph.ts b/ui/src/components/query_flamegraph.ts
index 019935c..a53898d 100644
--- a/ui/src/components/query_flamegraph.ts
+++ b/ui/src/components/query_flamegraph.ts
@@ -40,7 +40,7 @@
import type {Trace} from '../public/trace';
import {sqliteString} from '../base/string_utils';
import {parseUserFilterRegex} from '../widgets/flamegraph_regex';
-import {SharedAsyncDisposable} from '../base/shared_disposable';
+import type {SharedAsyncDisposable} from '../base/shared_disposable';
import {Monitor} from '../base/monitor';
export interface QueryFlamegraphColumn {
@@ -195,6 +195,8 @@
readonly unfilteredCumulativeValue: number;
}
+export type QueryFlamegraphDependency = SharedAsyncDisposable<AsyncDisposable>;
+
// A Perfetto UI component which wraps the `Flamegraph` widget and fetches the
// data for the widget by querying an `Engine`.
export class QueryFlamegraph implements AsyncDisposable {
@@ -212,9 +214,9 @@
constructor(
private readonly trace: Trace,
- dependencies: ReadonlyArray<AsyncDisposable> = [],
+ dependencies: ReadonlyArray<QueryFlamegraphDependency> = [],
) {
- this.dependencies = dependencies.map((d) => SharedAsyncDisposable.wrap(d));
+ this.dependencies = dependencies.map((d) => d.clone());
}
async [Symbol.asyncDispose](): Promise<void> {
@@ -260,11 +262,10 @@
const engine = this.trace.engine;
this.queryLimiter.schedule(async () => {
this.data = undefined;
- // Clone all the dependencies to make sure the the are not dropped while
- // this function is running, adding them to the trash to make sure they
- // are disposed after this function returns, but note this won't
- // actually drop the tables unless this class instances have also been
- // disposed due to the SharedAsyncDisposable logic.
+ // Clone all dependencies so they cannot be dropped while this function
+ // is running. Disposing these clones after the function returns does not
+ // drop the tables while either this instance or the caller still owns a
+ // clone.
await using trash = new AsyncDisposableStack();
for (const dependency of this.dependencies ?? []) {
// If the dependency is disposed, it means that we have already ended
diff --git a/ui/src/plugins/dev.perfetto.TraceProcessorTrack/index.ts b/ui/src/plugins/dev.perfetto.TraceProcessorTrack/index.ts
index 7666caa..cc1cefd 100644
--- a/ui/src/plugins/dev.perfetto.TraceProcessorTrack/index.ts
+++ b/ui/src/plugins/dev.perfetto.TraceProcessorTrack/index.ts
@@ -16,6 +16,8 @@
import {removeFalsyValues} from '../../base/array_utils';
import {AsyncLimiter} from '../../base/async_limiter';
import {ensureExists} from '../../base/assert';
+import {AsyncDisposableStack} from '../../base/disposable_stack';
+import {SharedAsyncDisposable} from '../../base/shared_disposable';
import {Time} from '../../base/time';
import {
createAggregationTab,
@@ -25,6 +27,7 @@
import {openDistributionTab} from '../../components/distribution_panel';
import {
metricsFromTableOrSubquery,
+ type QueryFlamegraphDependency,
type QueryFlamegraphMetric,
} from '../../components/query_flamegraph';
import {FlamegraphPanel} from '../../components/flamegraph_panel';
@@ -74,6 +77,11 @@
typeof TRACE_PROCESSOR_TRACK_PLUGIN_STATE_SCHEMA
>;
+interface SliceFlamegraphData extends AsyncDisposable {
+ readonly metrics: ReadonlyArray<QueryFlamegraphMetric>;
+ readonly dependencies: ReadonlyArray<QueryFlamegraphDependency>;
+}
+
function createDetailsPanel(trace: Trace, utid: number | null) {
if (utid === null) {
return undefined;
@@ -605,12 +613,7 @@
private createSliceFlameGraphPanel(trace: Trace) {
let previousSelection: AreaSelection | undefined;
- let computed:
- | {
- metrics: ReadonlyArray<QueryFlamegraphMetric>;
- dependencies: ReadonlyArray<AsyncDisposable>;
- }
- | undefined;
+ let computed: SliceFlamegraphData | undefined;
let isLoading = false;
const limiter = new AsyncLimiter();
@@ -624,8 +627,10 @@
previousSelection = selection;
if (selectionChanged) {
limiter.schedule(async () => {
+ const previousComputed = computed;
computed = undefined;
isLoading = true;
+ await previousComputed?.[Symbol.asyncDispose]();
computed = await this.computeSliceFlamegraph(trace, selection);
isLoading = false;
});
@@ -657,13 +662,7 @@
private async computeSliceFlamegraph(
trace: Trace,
currentSelection: AreaSelection,
- ): Promise<
- | {
- metrics: ReadonlyArray<QueryFlamegraphMetric>;
- dependencies: ReadonlyArray<AsyncDisposable>;
- }
- | undefined
- > {
+ ): Promise<SliceFlamegraphData | undefined> {
const trackIds = [];
for (const trackInfo of currentSelection.tracks) {
if (!trackInfo?.tags?.kinds?.includes(SLICE_TRACK_KIND)) {
@@ -698,11 +697,14 @@
},
});
- const iiTable = await createIITable(
- trace.engine,
- dataset,
- currentSelection.start,
- currentSelection.end,
+ await using disposables = new AsyncDisposableStack();
+ const iiTable = disposables.use(
+ await createIITable(
+ trace.engine,
+ dataset,
+ currentSelection.start,
+ currentSelection.end,
+ ),
);
// Will be automatically cleaned up when `iiTable` is dropped.
await createPerfettoIndex({
@@ -787,7 +789,14 @@
metrics,
);
});
- return {metrics, dependencies: [iiTable]};
+ const dependency: QueryFlamegraphDependency = SharedAsyncDisposable.wrap(
+ disposables.move(),
+ );
+ return {
+ metrics,
+ dependencies: [dependency],
+ [Symbol.asyncDispose]: () => dependency[Symbol.asyncDispose](),
+ };
}
private addMinimapContentProvider(ctx: Trace) {
