fix(dataset): finish teardown before releasing image data

Author: PaulHax

src/components/SliceSlider.vue

@@ -63,6 +63,7 @@ export default {
     return {
       maxHandlePos: 0,
       dragging: false,
+      pointerId: null,
       initialHandlePos: 0,
       initialMousePosY: 0,
       yOffset: 0,
@@ -96,27 +97,31 @@ export default {
   },
 
   beforeUnmount() {
-    this.resizeObserver.disconnect();
+    this.resizeObserver?.disconnect();
   },
 
   methods: {
     updateMaxHandlePos() {
+      if (!this.$refs.handleContainer) return;
       this.maxHandlePos =
         this.$refs.handleContainer.clientHeight - this.handleHeight;
     },
 
     onDragStart(ev) {
+      const container = this.$refs.handleContainer;
+      if (!container) return;
       ev.preventDefault();
 
       this.dragging = true;
+      this.pointerId = ev.pointerId;
       this.initialMousePosY = ev.pageY;
 
       if (ev.target === this.$refs.handle) {
         const handleStyles = window.getComputedStyle(this.$refs.handle);
         this.initialHandlePos = getYOffsetFromTransform(handleStyles.transform);
       } else {
         // move handle to mouse pos
-        const { y } = this.$refs.handleContainer.getBoundingClientRect();
+        const { y } = container.getBoundingClientRect();
         this.initialHandlePos = Math.max(
           0,
           Math.min(this.maxHandlePos, ev.pageY - y - this.handleHeight / 2)
@@ -127,11 +132,12 @@ export default {
 
       this.yOffset = 0;
 
-      this.$refs.handleContainer.setPointerCapture(ev.pointerId);
+      container.setPointerCapture(ev.pointerId);
     },
 
     onDragMove(ev) {
-      if (!this.$refs.handleContainer.hasPointerCapture(ev.pointerId)) return;
+      const container = this.$refs.handleContainer;
+      if (!container?.hasPointerCapture(ev.pointerId)) return;
       ev.preventDefault();
 
       this.yOffset = ev.pageY - this.initialMousePosY;
@@ -140,11 +146,18 @@ export default {
     },
 
     onDragEnd(ev) {
-      if (!this.$refs.handleContainer.hasPointerCapture(ev.pointerId)) return;
-      ev.preventDefault();
-      this.$refs.handleContainer.releasePointerCapture(ev.pointerId);
+      const container = this.$refs.handleContainer;
+      if (ev.pointerId !== this.pointerId) return;
+      const captured = container?.hasPointerCapture(ev.pointerId);
+      if (captured) {
+        ev.preventDefault();
+        container.releasePointerCapture(ev.pointerId);
+      }
 
       this.dragging = false;
+      this.pointerId = null;
+      if (!captured) return;
+
       const slice = this.getNearestSlice(this.handlePosition);
       this.$emit('update:modelValue', slice);
     },

src/components/__tests__/SliceSlider.spec.ts

@@ -0,0 +1,42 @@
+import { mount } from '@vue/test-utils';
+import { describe, expect, it, vi } from 'vitest';
+
+import SliceSlider from '@/src/components/SliceSlider.vue';
+
+const mountSlider = () =>
+  mount(SliceSlider, {
+    props: { min: 0, max: 10, step: 1 },
+  });
+
+describe('SliceSlider', () => {
+  it('ignores a late pointer event after its element is gone', () => {
+    const wrapper = mountSlider();
+    const vm = wrapper.vm as unknown as {
+      onDragMove: (event: PointerEvent) => void;
+    };
+    wrapper.unmount();
+
+    expect(() => vm.onDragMove({ pointerId: 7 } as PointerEvent)).not.toThrow();
+  });
+
+  it('keeps dragging when an unrelated pointer ends', async () => {
+    const wrapper = mountSlider();
+    const container = wrapper.element as HTMLElement;
+    const releasePointerCapture = vi.fn();
+    container.hasPointerCapture = vi.fn((pointerId) => pointerId === 7);
+    container.releasePointerCapture = releasePointerCapture;
+
+    await wrapper.trigger('pointerdown', { pointerId: 7, pageY: 10 });
+
+    const vm = wrapper.vm as unknown as {
+      dragging: boolean;
+      pointerId: number | null;
+      onDragEnd: (event: PointerEvent) => void;
+    };
+    vm.onDragEnd({ pointerId: 8 } as PointerEvent);
+
+    expect(vm.dragging).toBe(true);
+    expect(vm.pointerId).toBe(7);
+    expect(releasePointerCapture).not.toHaveBeenCalled();
+  });
+});

src/store/__tests__/datasetRemoveCascade.spec.ts

@@ -1,5 +1,6 @@
-import { beforeEach, describe, expect, it } from 'vitest';
+import { beforeEach, describe, expect, it, vi } from 'vitest';
 import { setActivePinia, createPinia } from 'pinia';
+import { nextTick } from 'vue';
 import vtkImageData from '@kitware/vtk.js/Common/DataModel/ImageData';
 import vtkDataArray from '@kitware/vtk.js/Common/Core/DataArray';
 
@@ -120,6 +121,25 @@ describe('dataset remove — synchronous reference cascade', () => {
     expect(viewStore.getViewsForData('img-1')).toEqual([]);
   });
 
+  it('detaches consumers before disposing the removed image', async () => {
+    seatImage('img-1', 'CT');
+    const imageCacheStore = useImageCacheStore();
+    const image = imageCacheStore.imageById['img-1'];
+    const dispose = vi.spyOn(image, 'dispose');
+    const viewStore = useViewStore();
+    bindFirstViewTo('img-1');
+
+    useDatasetStore().remove('img-1');
+
+    expect(imageCacheStore.imageById).not.toHaveProperty('img-1');
+    expect(viewStore.getViewsForData('img-1')).toEqual([]);
+    expect(dispose).not.toHaveBeenCalled();
+
+    await nextTick();
+
+    expect(dispose).toHaveBeenCalledOnce();
+  });
+
   it('drops crop state keyed by the removed image', () => {
     seatImage('img-1', 'CT');
     const cropStore = useCropStore();

src/store/__tests__/image-cache.spec.ts

@@ -0,0 +1,36 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+import { createPinia, setActivePinia } from 'pinia';
+import vtkDataArray from '@kitware/vtk.js/Common/Core/DataArray';
+import vtkImageData from '@kitware/vtk.js/Common/DataModel/ImageData';
+
+import { useImageCacheStore } from '@/src/store/image-cache';
+
+const seatImage = () => {
+  const data = vtkImageData.newInstance();
+  data.setDimensions(2, 2, 2);
+  data.getPointData().setScalars(
+    vtkDataArray.newInstance({
+      name: 'scalars',
+      numberOfComponents: 1,
+      values: new Uint8Array(8),
+    })
+  );
+  const store = useImageCacheStore();
+  store.addVTKImageData(data, 'CT', { id: 'img-1' });
+  return store;
+};
+
+describe('image cache lifecycle', () => {
+  beforeEach(() => {
+    setActivePinia(createPinia());
+  });
+
+  it('treats an image whose VTK data is unavailable as absent', () => {
+    const store = seatImage();
+    vi.spyOn(store.imageById['img-1'], 'getVtkImageData').mockReturnValue(
+      undefined as never
+    );
+
+    expect(store.getVtkImageData('img-1')).toBeNull();
+  });
+});

src/store/datasets-images.ts

@@ -51,8 +51,8 @@ export const useImageStore = defineStore('images', () => {
   }
 
   function deleteData(id: string) {
-    useImageCacheStore().removeImage(id);
     removeFromArray(idList.value, id);
+    useImageCacheStore().removeImage(id);
   }
 
   function checkAllImagesSameSpace() {

src/store/datasets.ts

@@ -256,10 +256,12 @@ export const useDatasetStore = defineStore('dataset', () => {
     // Anonymous volume.
     loadedData.value = loadedData.value.filter((d) => d.dataID !== id);
     dicomStore.deleteVolume(id);
-    imageStore.deleteData(id);
     layersStore.remove(id);
     useViewConfigStore().removeData(id);
     useImageStatsStore().removeData(id);
+    // Cache eviction disposes the VTK object after Vue has flushed consumers.
+    // Run every other synchronous reference cleanup before starting eviction.
+    imageStore.deleteData(id);
   };
 
   const removeAll = () => {

src/store/image-cache.ts

@@ -9,7 +9,7 @@ import { Maybe } from '@/src/types';
 import { ImageMetadata } from '@/src/types/image';
 import vtkImageData from '@kitware/vtk.js/Common/DataModel/ImageData';
 import { defineStore } from 'pinia';
-import { markRaw, reactive, ref } from 'vue';
+import { markRaw, nextTick, reactive, ref } from 'vue';
 
 /**
  * An internal cache of progressively loadable images.
@@ -37,7 +37,7 @@ export const useImageCacheStore = defineStore('image-cache', () => {
     const data = image.getVtkImageData();
     // ProgressiveImage initializes with empty vtkImageData before actual data loads.
     // VTK.js volume renderer crashes on empty data (null scalar texture).
-    if (!data.getPointData().getScalars()?.getData()?.length) return null;
+    if (!data?.getPointData().getScalars()?.getData()?.length) return null;
     return data;
   }
 
@@ -116,19 +116,20 @@ export const useImageCacheStore = defineStore('image-cache', () => {
 
   function removeImage(id: string) {
     if (!(id in imageById)) return;
+    const image = imageById[id];
     unregisterListeners(id);
 
-    // Release vtk data and any per-image caches (e.g. cine compressed frames
-    // and decoded-frame LRU). Without this, removing a dataset leaks all of
-    // its memory until the page reloads.
-    imageById[id].dispose();
-
     const idx = imageIds.value.indexOf(id);
     if (idx > -1) imageIds.value.splice(idx, 1);
     delete imageById[id];
     delete imageStatus[id];
     delete imageLoading[id];
     delete imageErrors[id];
+
+    // Vue tears down image consumers in its next update flush. Keep the VTK
+    // object alive until those consumers have detached their actors and event
+    // handlers, but make it unreachable from the cache immediately.
+    void nextTick(() => image.dispose());
     [...deletionCallbacks].forEach((callback) => callback([id]));
   }