Skip to content

fix(core): remove IMAGE_CACHE_IMAGE_ADDED listener when streaming image load settles - #2809

Open
magang0425 wants to merge 1 commit into
cornerstonejs:mainfrom
magang0425:fix/streaming-volume-cache-listener-leak
Open

fix(core): remove IMAGE_CACHE_IMAGE_ADDED listener when streaming image load settles#2809
magang0425 wants to merge 1 commit into
cornerstonejs:mainfrom
magang0425:fix/streaming-volume-cache-listener-leak

Conversation

@magang0425

@magang0425 magang0425 commented Jul 16, 2026

Copy link
Copy Markdown

BaseStreamingImageVolume.callLoadImage registers an IMAGE_CACHE_IMAGE_ADDED
listener on the global eventTarget so the VTK texture frame gets updated when
the loaded image lands in the image cache. The listener is only removed inside
the handler itself, i.e. when an image with a matching imageId actually
arrives.

However, streaming volume loads typically bypass the image cache entirely
(transferPixelData: true — see the comment above callLoadImage), so for the
common path the event never fires and the listener is never removed:

  • one listener leaks per frame request — a few CBCT/MR volumes with hundreds of
    slices each accumulate thousands of listeners on the global eventTarget;
  • each leaked closure captures this, so decached volumes are retained and can
    never be garbage collected, which turns into a significant memory leak in
    long-running viewers that swap volumes.

Summary by CodeRabbit

  • Bug Fixes

    • Improved image-loading event listener cleanup after successful loads and errors.
    • Prevented cache events from updating image frames after loading has completed.
    • Preserved real-time frame updates during active image loading while ignoring unrelated images.
  • Tests

    • Added coverage for successful loads, rejected loads, pending loads, and event filtering.

…ge load settles

callLoadImage registers a listener on the global eventTarget to update the
VTK texture when the loaded image is added to the image cache, but only
removes it when a matching image actually arrives. Streaming volume loads
typically bypass the image cache, so every frame request leaked one
listener, and each closure retained the volume instance, preventing
decached volumes from being garbage collected.

Remove the listener whenever the load settles (resolve, reject, or
synchronous throw). Cache adds that arrive while the request is in flight
still update the texture frame as before.
@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 2c216631-7b35-4c82-8758-0ad99caba726

📥 Commits

Reviewing files that changed from the base of the PR and between 8ae82b6 and cca7cd5.

📒 Files selected for processing (2)
  • packages/core/src/cache/classes/BaseStreamingImageVolume.ts
  • packages/core/test/BaseStreamingImageVolume_listenerCleanup.jest.js

📝 Walkthrough

Walkthrough

BaseStreamingImageVolume.callLoadImage now centralizes cache listener cleanup for successful and failed loads. Tests cover listener removal, matching in-flight cache updates, image ID filtering, and callback behavior.

Changes

Image cache listener cleanup

Layer / File(s) Summary
Listener cleanup flow and lifecycle validation
packages/core/src/cache/classes/BaseStreamingImageVolume.ts, packages/core/test/BaseStreamingImageVolume_listenerCleanup.jest.js
Listener removal is centralized and performed after successful or failed loading, while matching cache events can update frames during an in-flight load. Jest tests validate resolution, rejection, event filtering, callback invocation, and post-settlement cleanup.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning It includes Context, but the required Changes & Results, Testing, and Checklist sections are missing or incomplete. Add the missing template sections, complete the PR checklist, and include testing steps plus a brief before/after summary.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is specific, concise, and accurately summarizes the main change to streaming image listener cleanup.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.

1 participant