fix: prevent favicon data retention in heartbeat debug logs#223
Conversation
The console.debug calls passed entire Tab objects, which in Firefox include favIconUrl as base64 data URIs. Browser console buffers retain references to logged objects, so over thousands of heartbeats this accumulated hundreds of MBs of duplicate favicon data in memory. Fix: destructure only needed fields from Tab objects before use, and log only the URL string instead of the full Tab object. This addresses the favicon accumulation half of ActivityWatch#222 (the AbortController leak is being fixed in aw-client-js via ActivityWatch/aw-client-js#50).
Greptile SummaryThis PR fixes a significant memory leak in Firefox where Confidence Score: 5/5Safe to merge — the core memory leak fix is correct and complete; only a minor over-removal of small safe data from two internal debug logs. All findings are P2. The listener-level debug log changes correctly eliminate Tab object retention in the console buffer. The only concern is that previousData and data (neither of which contain favicon content) were also removed from two internal log calls, reducing debuggability without fixing any memory issue — but this does not affect runtime correctness or the leak fix itself. No files require special attention beyond the single P2 suggestion on heartbeat.ts. Important Files Changed
Sequence DiagramsequenceDiagram
participant Listener as Listener (alarm/tab/init)
participant HB as heartbeat()
participant Console as console.debug buffer
participant GC as Garbage Collector
Note over Listener,GC: BEFORE fix
Listener->>Console: console.debug('...', tab) — retains Tab + favIconUrl
Listener->>HB: heartbeat(client, tab, tabCount)
HB->>Console: console.debug('Sending heartbeat', data)
Note over Console: Tab object pinned in buffer<br/>favIconUrl (~20 KB) × thousands = ~761 MB leak
Note over Listener,GC: AFTER fix
Listener->>Console: console.debug('...', tab.url) — retains only string
Listener->>HB: heartbeat(client, tab, tabCount)
HB-->>GC: Tab object eligible for GC after function returns
HB->>Console: console.debug('Sending heartbeat', url)
Note over Console: Only URL strings retained — no favicon data
Reviews (1): Last reviewed commit: "fix: prevent favicon data retention in h..." | Re-trigger Greptile |
| @@ -46,7 +51,7 @@ async function heartbeat( | |||
| config.heartbeat.intervalInSeconds + 20, | |||
| ) | |||
| } | |||
| console.debug('Sending heartbeat', data) | |||
| console.debug('Sending heartbeat', url) | |||
There was a problem hiding this comment.
Over-removal of debug context for small data objects
previousData and data are both plain IEvent['data'] objects containing only url, title, audible, incognito, and tabCount — no favIconUrl or other large base64 content. Removing them from these two log calls reduces debugging utility (e.g., you can no longer see what changed between heartbeats, or what audible/incognito state was sent) without addressing any memory issue. The leak came from logging the full Tab object in the listener functions, not from logging data or previousData.
| console.debug('Sending heartbeat for previous data', previousData) |
And on line 54:
| console.debug('Sending heartbeat', data) |
There was a problem hiding this comment.
Good catch. and only contain small fields (url, title, audible, incognito, tabCount) — no favicon data. Restoring them is the right call for debuggability. Fixed in bb10813.
|
@TimeToBuildBob Valid concern in #223 (comment) |
These objects contain only small fields (url, title, audible, incognito, tabCount) — no favIconUrl or large data. Removing them was unnecessary and reduced debugging utility without fixing any memory issue. The memory leak came from logging full Tab objects in the listener functions, not from these internal heartbeat log calls. Addresses Greptile review feedback on ActivityWatch#223.
Problem
The heartbeat module passes entire
Tabobjects toconsole.debug(). In Firefox,Tab.favIconUrlis a base64 data URI (often 10-20 KB). Browser console buffers retain references to logged objects, so over thousands of heartbeats the retainedTabreferences accumulate hundreds of MBs of duplicate favicon data in memory.From the
about:memoryreport in #222:Fix
url,title,audible,incognitofrom theTabobject, so the full object (includingfavIconUrl) can be GC'd immediatelyconsole.debug('...', tab)withconsole.debug('...', tab.url)— retains useful debug info without retaining the full objectThis is the favicon half of #222. The AbortController leak (the other half, ~123 MB of leaked controllers + ~645 MB of closures) is being addressed in aw-client-js via ActivityWatch/aw-client-js#50.
Test plan
npm run build)about:memoryno longer shows large favicon string accumulation under the extension's background page