[dfliess]

Four derived callbacks subscribe to an inner store without returning the unsubscriber, so every re-run of the outer store leaves the previous subscription alive, and the last one stays alive after unsubscribing:

• canvas/components/markdown/util.ts: getResolveTemplatedStringQueryOptions adds one live subscription to the parent canvas spec on every filter change.
• models/selectors.ts and sources/selectors.ts: the column queries for every model and source, re-created each time the list changes.
• time-series/totals-data-store.ts: createUnfilteredTotalsForMeasure keeps the previous aggregation query observed each time the filters or the time range change.

The fix is the same one line in each: return the inner subscription. markdown/util.spec.ts changes the filters twice and checks the parent spec is released on unsubscribe; the other three are covered by the existing suites.

This PR started as a fix for canvas components showing "No valid component" after a revisit; #10024 fixed that root cause by evicting disposed entities, so that part was dropped.

---

Developed in collaboration with Claude Code

[dfliess]

While reviewing this, we found the same missing return (the second commit here) in three other places. Each one subscribes to an inner store inside a derived callback but doesn't return the unsubscriber, so the previous subscription stays alive every time the outer store re-runs, and the last one stays alive after unsubscribing:

• models/selectors.ts#L39-L52: the column queries for every model, re-created each time the model list changes.
• sources/selectors.ts#L87-L102: the same for sources.
• time-series/totals-data-store.ts#L61-L82: createUnfilteredTotalsForMeasure keeps the previous aggregation query observed each time the filters or the time range change.

We haven't reproduced these in the app, only read the code. The fix would be the same one line each (return derived(...) / return createQueryService...(...)). Would you like them in this PR, in a separate one, or would you rather handle them yourselves?

---

Developed in collaboration with Claude Code

[AdityaHegde]

Thanks for the analysis. Can you add the return to the place you found?

[AdityaHegde]

How about reassigning it to useCanvas in acquire() to keep this simple?

[dfliess]

I tried that, and it fixes the components but loses the theme: the constructor builds this.theme from this.specStore, so after the spec query is garbage collected the theme stays on the old store and a canvas with its own theme comes back with the default one. I added a test for it in CanvasSpecGarbageCollection.spec.ts ("Should keep the canvas theme...").

It can be done your way if acquire() recreates both, with a createSpecStores() that builds specStore and theme and is called from the constructor and acquire(). It's on a separate branch so you can compare, and it passes the same tests: dfliess/rill@dfliess/canvas-spec-store-gc...dfliess/canvas-spec-store-gc-acquire

I kept the readable because it gives one stable store, so anything built from it, today or later, keeps working after gc without having to be recreated in acquire(). Happy to switch if you prefer the explicit version.

[AdityaHegde]

I think the explicit version makes more sense. We will rethink the lifecycle to make sure acquire() is called, it is right now.

[dfliess]

#10024 fixed the root cause in the meantime by evicting disposed entities, so a revisit always builds a new entity and neither version of this change is needed. I dropped it, along with the theme test. The PR now only has the subscription leaks: the markdown template one and the three returns you asked for. If you'd rather have them in a fresh PR, I can close this one and open a new one.

[AdityaHegde]

Thanks, we can reuse this PR itself.

[dfliess]

Added the return in the three places (models/selectors.ts, sources/selectors.ts, totals-data-store.ts) and replied inline about acquire().