Skip to content

Fix memory leak: return early on image cache hit in MessageImageView#93

Merged
viktorstrate merged 1 commit intoviktorstrate:mainfrom
beezly:fix/image-cache-memory-leak-v2
Apr 9, 2026
Merged

Fix memory leak: return early on image cache hit in MessageImageView#93
viktorstrate merged 1 commit intoviktorstrate:mainfrom
beezly:fix/image-cache-memory-leak-v2

Conversation

@beezly
Copy link
Copy Markdown
Contributor

@beezly beezly commented Apr 7, 2026

Summary

Fixes the memory leak where getMediaContent was called on every channel visit even for already-cached images.

Two-layer caching design

NSCache (checked in init): synchronously pre-populates image before the view appears, preventing flicker on scroll/revisit. Also skips the expensive decode step (toOrientedImage) on repeat loads.

SDK media cache (getMediaContent): avoids network re-fetches. Now always called in .task so imageData is populated for drag-and-drop, regardless of whether the decoded NSImage is already cached.

The image is already on screen (via init) before .task runs its disk read, so flicker prevention is unaffected.

Re: drag-and-drop regression

The original version of this PR returned early on a cache hit without populating imageData, breaking drag-and-drop. This revision always calls getMediaContent to ensure imageData is set, but still skips the decode step when the NSImage is already in the cache.

Test plan

  • Navigate to a channel with images — images load without flicker
  • Switch channels and back — no memory growth, images still appear immediately
  • Drag an image to the desktop on first load (cache miss) — file is copied correctly
  • Drag an image to the desktop after switching channels and back (cache hit) — file is copied correctly

Fixes #89

@viktorstrate
Copy link
Copy Markdown
Owner

This is a nice fix generally, but it breaks drag and drop to download images. If you try to drag an image to the desktop it will not copy the file, because the imageData variable is never set.

For preview, it still works, since the previewImage function will download the image if imageData is not set.
I think we can do something similar for drag and drop maybe?

The .task block checked NSCache on cache hit but never returned, causing
getMediaContent to be called on every channel visit even for already-cached
images.

The fix always fetches raw data via the SDK (which hits its own disk cache
on repeat loads, avoiding network requests), then skips the expensive decode
step (toOrientedImage) when the NSImage is already in the NSCache.

Two caching layers serve different purposes:
- NSCache (checked in init): synchronously pre-populates `image` before the
  view appears, preventing flicker on scroll/revisit. The image is already
  on screen before .task runs its disk read.
- SDK media cache (getMediaContent): avoids network re-fetches. Always
  called so `imageData` is populated for drag-and-drop, regardless of
  whether the decoded NSImage is cached.

Fixes viktorstrate#89
@beezly beezly force-pushed the fix/image-cache-memory-leak-v2 branch from 2c666c1 to 8dfbbee Compare April 8, 2026 08:11
@viktorstrate viktorstrate merged commit 05048e4 into viktorstrate:main Apr 9, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Memory leak when loading timelines.

2 participants