Repository navigation
fix(tui): keep render owner subscriptions weak - #220
Merged
Merged
Conversation
A long-lived shared child retained every discarded parent through the change-notification registry, including their rendered subtrees. Store all owners weakly and prune expired registrations without changing notification behavior. Explicit teardown alone cannot handle parents discarded without detaching their children.
Shrub24
requested review from
Kushalkhemka and
opensec-intelligence
as code owners
October 31, 2026 16:30
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.
What and why
After a long session, /new can leave old rendered message trees in memory even after GC. A shared component that survives the session switch retains its former rendering parents through renderOwners, including their children and cached output. Repeated render-tree rebuilds can therefore accumulate objects that are no longer mounted.
This also affects extension-provided UI if it reuses a child component across discarded parents. It is not a general leak of session-scoped extension state; the fix removes this specific render-registry retaining path. In one observed session an 800mb heap peak of retained renderer components accumulated through multiple new sessions; with this fix it returns to baseline on /new.
The mechanism:
renderOwnersuses weak keys but strong owner values. As long as a shared child remains live, it retains discarded parents and their cached render trees. For example, repeatedly rendering a cachedTextinside newBoxinstances keeps those boxes alive after the application drops them.Store both singleton and multiple-owner subscriptions as
WeakRefs, pruning dead registrations during registration, removal and change notification. Live parents still receive updates; discarded parents no longer need explicit teardown to become collectible. The public API is unchanged.The TUI README documents the ownership rule. This leaves the separate
Container.clear()render-cache behavior unchanged.How it was tested
--expose-gcverifies collection of one singleton owner and 100 owners of a shared child after an event-loop turn. It also checks that a surviving cached ancestor receives the next change. The collection test fails with the original implementation and passes with this fix.npm run checkpasses on the rebased branch.The full monorepo suite has not been rerun for this change. No interleaved
bench/speed or memory measurements have been collected; the regression measures collectibility, not RSS savings or rendering overhead.Checklist
npm run checkis clean and./test.shpasses (checkpasses; full suite not rerun)bench/are abovetests/aot/run.shandtests/pi/run.shpass on a build with it (not applicable)CHANGELOG.mdedits