Skip to content

Fix infinite memory usage by allowing old data to be garbage collected#2591

Open
alerque wants to merge 4 commits into
simonmichael:hledger1from
alerque:ui-leak
Open

Fix infinite memory usage by allowing old data to be garbage collected#2591
alerque wants to merge 4 commits into
simonmichael:hledger1from
alerque:ui-leak

Conversation

@alerque

@alerque alerque commented May 16, 2026

Copy link
Copy Markdown
Collaborator

Re-opening of #2473. Fixes #1825.

@simonmichael

Copy link
Copy Markdown
Owner

Hi @alerque, sorry for the delay on this. Have you been using it much ?

I got these warnings from opus:

1. Regression: regenerateScreens no longer updates aPrevScreens

Old UIState.hs:381:

  ui{ajournal=j, aScreen=screenUpdate opts d j s, aPrevScreens=map (screenUpdate opts d j) ss}

New:

  let !ui' = ui{ajournal=j, aScreen=s'}
      !s' = case s of ...

aPrevScreens is silently dropped from regeneration. After reload on a
deep screen, popping back will show stale data. The XXX comment was
edited to only mention the error screen, but the breakage is wider —
every prev screen on the stack is now stale. This needs to be restored
(with the same $!-style strictness, if desired).

2. Redundant regeneration in rsHandle

RegisterScreen.hs:248:

  e | e `elem` [...] -> do
    ui' <- uiReload copts d ui
    put' $ regenerateScreens (ajournal ui') d ui'

uiReload already calls regenerateScreens j d ui internally
(ErrorScreen.hs:177), so this regenerates twice on every g press /
file change — exactly the hot path the PR is trying to optimize. And
if reload failed (error screen pushed), this re-regenerates against
the old journal. Should just be uiReload copts d ui >>= put' like the
other screens.

3. Top-level type signature removed

UIScreens.hs:245:

  -rsUpdate :: UIOpts -> Day -> Journal -> RegisterScreenState -> RegisterScreenState
  -rsUpdate uopts d j rss@RSS{_rssAccount, _rssForceInclusive, _rssList=oldlist} =
  +rsUpdate uopts d j rss@RSS{_rssAccount, _rssForceInclusive} =

-Wall will warn here.

4. tsUpdate fallback may show stale data

  updatedTxn = case find (\t -> tindex t == tindex currentTxn) (jtxns j) of
    Just t  -> t
    Nothing -> currentTxn  -- fallback to current if not found

If the user deletes the displayed transaction externally and
reloads, the screen will silently keep showing the old
transaction. The XXX caveat comment acknowledges index shifts but
not deletions. Worth considering popping back to the register screen
on Nothing instead

@simonmichael simonmichael added ui The hledger-ui tool. needs-testing To unblock: needs more developer testing or general usage needs-review To unblock: needs more code/docs/design review by someone labels Jun 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-review To unblock: needs more code/docs/design review by someone needs-testing To unblock: needs more developer testing or general usage ui The hledger-ui tool.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

hledger-ui --watch consumes more CPU and RAM over time [$150 x 3]

2 participants