Skip to content

Fix: cannot load segmentation after export in local mode #5999

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nithin-trenser wants to merge 3 commits into
base: master
Choose a base branch
from
+45 −34
Open

Fix: cannot load segmentation after export in local mode #5999

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d0c7aee
Fix: cannot load segmentation after export in local mode
nithin-trenser May 7, 2026
e3da11c
fix: prevent shared metadata mutation in combineFrameInstance
trenser-belbin May 21, 2026
cf98514
Merge branch 'master' into fix-segmentation-load-error
Belbin-GK May 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions extensions/cornerstone-dicom-seg/src/viewports/OHIFCornerstoneSEGViewport.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,10 +292,12 @@ function _getReferencedDisplaySetMetadata(referencedDisplaySet, segDisplaySet) {

const { PixelMeasuresSequence } = SharedFunctionalGroup;

const PixelMeasures = Array.isArray(PixelMeasuresSequence)
? PixelMeasuresSequence[0]
: PixelMeasuresSequence;
const PixelMeasures =
(Array.isArray(PixelMeasuresSequence) ? PixelMeasuresSequence[0] : PixelMeasuresSequence) || {};

if (!PixelMeasuresSequence || Object.keys(PixelMeasures).length === 0) {
console.warn('PixelMeasuresSequence missing from SEG instance metadata.');
}
const { SpacingBetweenSlices, SliceThickness } = PixelMeasures;

const image0 = referencedDisplaySet.images[0];
Expand Down
8 changes: 7 additions & 1 deletion platform/core/src/classes/MetadataProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,13 @@ class MetadataProvider {
return;
}

return (frameNumber && combineFrameInstance(frameNumber, instance)) || instance;
const combined = frameNumber && combineFrameInstance(frameNumber, instance);
if (combined) {
// Add imageId to multiframe result so it matches single-frame instance.
combined.imageId = imageId;
return combined;
}
Comment on lines +63 to +68
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot May 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Mutation of a cached combined instance

createCombinedValue inside combineFrameInstance caches the per-frame object at instance._parentInstance[frameNumber] and returns the same reference on every subsequent call. Setting combined.imageId = imageId permanently stamps that imageId onto the cached object. If the same SOP-instance frame is ever resolved through two different imageIds (e.g., the same DICOM served from two WADO-RS endpoints), the second caller receives a combined instance whose imageId belongs to the first caller.

Additionally, when NumberOfFrames < 2 combineFrameInstance returns the original instance reference unchanged. If frameNumber is somehow truthy for such an instance, combined === instance, and combined.imageId = imageId mutates the shared object stored in DicomMetadataStore directly — not just its per-frame projection.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: platform/core/src/classes/MetadataProvider.ts
Line: 63-68

Comment:
**Mutation of a cached combined instance**

`createCombinedValue` inside `combineFrameInstance` caches the per-frame object at `instance._parentInstance[frameNumber]` and returns the same reference on every subsequent call. Setting `combined.imageId = imageId` permanently stamps that `imageId` onto the cached object. If the same SOP-instance frame is ever resolved through two different imageIds (e.g., the same DICOM served from two WADO-RS endpoints), the second caller receives a combined instance whose `imageId` belongs to the first caller.

Additionally, when `NumberOfFrames < 2` `combineFrameInstance` returns the original `instance` reference unchanged. If `frameNumber` is somehow truthy for such an instance, `combined === instance`, and `combined.imageId = imageId` mutates the shared object stored in `DicomMetadataStore` directly — not just its per-frame projection.

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Copy Markdown
Contributor

@Belbin-GK Belbin-GK May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed by avoiding mutation of cached/shared metadata objects in combineFrameInstance.

Previously, combined.imageId = imageId modified shared references returned from createCombinedValue, which could leak imageId values across callers. For single-frame instances, it could also mutate the original metadata object stored in DicomMetadataStore.

Updated the implementation to return a cloned object before assigning imageId.

return instance;
}

get(query, imageId, options = { fallback: false }) {
Expand Down
Loading