From 0508b80216cf5100b2c2ee7bcf9bd7a3663a6acb Mon Sep 17 00:00:00 2001 From: TrevorBurgoyne Date: Tue, 11 Aug 2026 16:58:51 -0500 Subject: [PATCH 1/6] add destroy to better cleanup memory leaks --- .github/tasks.md | 9 -- CHANGELOG.md | 9 ++ api_spec.md | 23 ++++- index.d.ts | 5 ++ package.json | 2 +- src/configuration.ts | 2 + src/index.js | 145 +++++++++++++++++++++++++++++- src/listeners.ts | 8 +- src/version.js | 2 +- tests/teardown.test.js | 194 +++++++++++++++++++++++++++++++++++++++++ 10 files changed, 384 insertions(+), 15 deletions(-) create mode 100644 tests/teardown.test.js diff --git a/.github/tasks.md b/.github/tasks.md index 162d72e3..826c5841 100644 --- a/.github/tasks.md +++ b/.github/tasks.md @@ -1,12 +1,3 @@ ## Tasks -### Bitmask segmentation annotation mode -Per-annotation binary masks, COCO-style RLE serialization, brush + erase interaction. - -- [x] Phase 1: Data model + RLE utils + tests (`src/mask_utils.ts`, `ULabelSpatialType`, `SPATIAL_TYPE_SET`) -- [x] Phase 2: Bitmask rendering layer (`draw_bitmask`, dispatch, redraw/clear) -- [x] Phase 3: Brush/erase paints pixels (begin/continue/finish for bitmask) -- [x] Phase 4: Undo/redo patch diffs for brush strokes (single-stroke `bitmask_stroke` action) -- [x] Phase 5: Toolbox + mode registration (mode button, brush enable/disable, keybinds) -- [ ] Phase 6: Export/import round-trip + tests + demo page diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f4ab387..9d929e27 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,15 @@ All notable changes to this project will be documented here. ## [unreleased] +## [0.26.0] - Aug 11th, 2026 +- **Memory leak fix on teardown.** Bitmask annotations attach a decoded pixel `Uint8Array` (`_mask`) and a tinted stencil canvas (`_mask_render`) to each annotation object. These persisted after `remove_listeners()`, and consumers that rebuild ULabel per navigation could accumulate multi-GB retained heap. Changes: + - New `destroy()` method on `ULabel`. Idempotent; releases per-annotation bitmask caches, empties action/undo streams, breaks toolbox back-references, clears the resize-observer array, and wipes the container DOM. Callers should prefer `destroy()` over `remove_listeners()` going forward. + - `destroy_annotation_context()` now also drops `_mask` / `_mask_render` / `_bitmask_box_hint` on the destroyed annotation, so `set_annotations()` also frees mask memory when it recycles contexts. + - New `auto_destroy_on_detach` config option (default `true`). When enabled, ULabel installs a `MutationObserver` on the container's root and calls `destroy()` automatically after the container leaves the DOM. + - `set_annotations()`, `get_annotations()`, and `redraw_all_annotations()` now short-circuit with a warning if called after `destroy()`. + - Fixed a latent typo in `set_annotations()` that wrote to a non-existent `undo_stack` property instead of clearing the real `undone_stack`. + - `remove_ulabel_listeners()` now scopes its `.id_dialog` cleanup to the instance's container, so tearing down one ULabel no longer strips id-dialog handlers from siblings on the same page. + ## [0.25.1] - Aug 10th, 2026 - The `ConfidenceSlider` toolbox item now also filters `bitmask` annotations diff --git a/api_spec.md b/api_spec.md index f302894b..7308ef3f 100644 --- a/api_spec.md +++ b/api_spec.md @@ -79,7 +79,8 @@ class ULabel({ annotation_size_minus_keybind: string, annotation_vanish_keybind: string, fly_to_max_zoom: number, - n_annos_per_canvas: number + n_annos_per_canvas: number, + auto_destroy_on_detach: boolean }) ``` @@ -615,6 +616,9 @@ If `true`, the user can click and drag to contiuously place points for polyline ### `allow_annotations_outside_image` When `false`, new annotations will be limited to points within the image, and attempts to move annotations outside the image will bounce back to inside the image. Default is `true`. +### `auto_destroy_on_detach` +When `true` (the default), ULabel installs a `MutationObserver` on the container's root and calls [`destroy()`](#destroy) automatically after the container is removed from the DOM. The observer holds the ULabel instance through a `WeakRef` (so it cannot pin the instance in memory on its own) and defers the teardown decision by one animation frame so brief detach/reattach cycles (portals, jQuery `.detach()`, layout reparenting) do not trigger a false-positive teardown. Set to `false` to opt out and manage teardown manually via [`destroy()`](#destroy). + ## Display Utility Functions @@ -653,6 +657,23 @@ Display utilities are provided for a constructed `ULabel` object. *() => void* -- Removes persistent event listeners from the document and window. Listeners attached directly to html elements are not explicitly removed. Note that ULabel will not function properly after this method is called. Designed for use in single-page applications before navigating away from the annotation page. +> Prefer [`destroy()`](#destroy) for new code — it also releases the heavy per-annotation bitmask caches and the container DOM. + +### `destroy()` + +*() => void* -- Fully tears down this ULabel instance. Idempotent (subsequent calls are no-ops). Releases: + +- per-bitmask runtime state (`_mask` `Uint8Array`, `_mask_render` tinted stencil canvas, `_bitmask_box_hint`), +- action stream and redo stack (which retain per-stroke `before_rle` / `after_rle` payloads), +- toolbox item back-references to the instance, +- resize observers and (if [`auto_destroy_on_detach`](#auto_destroy_on_detach) installed one) the auto-teardown `MutationObserver`, +- pending toast/interaction timers, +- all DOM under the configured container. + +After calling `destroy()` the instance MUST NOT be used again. `set_annotations()`, `get_annotations()`, and `redraw_all_annotations()` short-circuit with a warning if called on a destroyed instance. + +With [`auto_destroy_on_detach`](#auto_destroy_on_detach) enabled (the default), `destroy()` is called automatically after the container is removed from the DOM. Callers that opt out of the auto path should call `destroy()` explicitly during their unmount / teardown. + ### `fly_to_next_annotation(increment)` Sets the zoom to focus on a non-deprecated, spatial annotation in the active subtask's ordering that is an `` number away from the previously focused annotation, if any. Returns `true` on success and `false` on failure (eg, no valid annotations exist, or an annotation is currently actively being edited). diff --git a/index.d.ts b/index.d.ts index 1d119abb..ba2f8950 100644 --- a/index.d.ts +++ b/index.d.ts @@ -439,6 +439,11 @@ export class ULabel { // Listeners public remove_listeners(): void; + // Full teardown: releases bitmask caches, action streams, toolbox refs, and container DOM. + public destroy(): void; + // True after destroy() has run; subsequent destroy() calls are no-ops. + is_destroyed: boolean; + // Static functions static version(): string; static get_time(): string; diff --git a/package.json b/package.json index f16f467e..4cd503e8 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "ulabel", "description": "An image annotation tool.", - "version": "0.25.1", + "version": "0.26.0", "main": "dist/ulabel.min.js", "module": "dist/ulabel.min.js", "types": "dist/index.d.ts", diff --git a/src/configuration.ts b/src/configuration.ts index 5e9c620a..8e95aec3 100644 --- a/src/configuration.ts +++ b/src/configuration.ts @@ -276,6 +276,8 @@ export class Configuration { public allow_annotations_outside_image: boolean = true; + public auto_destroy_on_detach: boolean = true; + constructor(...kwargs: { [key: string]: unknown }[]) { this.modify_config(...kwargs); } diff --git a/src/index.js b/src/index.js index 6936daff..c40fe0fb 100644 --- a/src/index.js +++ b/src/index.js @@ -138,6 +138,79 @@ export class ULabel { remove_ulabel_listeners(this); } + /** + * Fully tear down this ULabel instance and release its heavy runtime state + * (bitmask masks, tinted stencil canvases, undo/redo streams, toolbox refs, + * DOM under the container). After calling this the instance MUST NOT be used + * again. Callers with a strong reference to the instance no longer keep the + * bitmask `Uint8Array`s alive. Safe to call more than once. + */ + destroy() { + if (this.is_destroyed) return; + + // 1. Disconnect the MutationObserver first so a mid-teardown throw can't + // leave a live observer pinning `this` via the browser's observer registry. + if (this.mutation_observer != null) { + this.mutation_observer.disconnect(); + this.mutation_observer = null; + } + + // 2. Existing listener / observer cleanup. + this.remove_listeners(); + + // 3. Any pending toast/interaction timers hold `this` in their closure. + if (this.annotation_navigation_toast_timeout !== null) { + clearTimeout(this.annotation_navigation_toast_timeout); + this.annotation_navigation_toast_timeout = null; + } + // In-progress bitmask move snapshot canvas (skipped by end_bitmask_move on interrupt). + if (this.state) { + this.state["bitmask_move_overlay"] = null; + } + + // 4. Drop bitmask caches on every annotation, in every subtask, and clear the + // action streams so retained RLE payloads (before/after) are collectible. + for (const subtask of Object.values(this.subtasks ?? {})) { + const access = subtask?.annotations?.access ?? {}; + for (const anno of Object.values(access)) { + delete anno["_mask"]; + delete anno["_mask_render"]; + delete anno["_bitmask_box_hint"]; + } + subtask.annotations = { ordering: [], access: {} }; + if (subtask.actions) { + subtask.actions.stream = []; + subtask.actions.undone_stack = []; + } + if (subtask.state) { + subtask.state["annotation_contexts"] = {}; + } + } + + // 5. Break the toolbox <-> ulabel back-reference. Toolbox items keep a + // `this.ulabel` and are stored on `this.toolbox.items`. + if (this.toolbox?.items) { + for (const item of this.toolbox.items) { + if (item) item.ulabel = null; + } + this.toolbox.items.length = 0; + } + this.toolbox = null; + + // 6. Drop the resize-observer array so disconnected observers stop retaining state. + this.resize_observers = []; + + // 7. Wipe the container DOM (canvases, id dialogs, brush circle, enders, overlays). + const container_id = this.config?.["container_id"]; + if (container_id) { + const container = document.getElementById(container_id); + if (container) container.innerHTML = ""; + } + + this.is_init = false; + this.is_destroyed = true; + } + static process_allowed_modes(ul, subtask_key, subtask) { // TODO(v1) check to make sure these are known modes ul.subtasks[subtask_key]["allowed_modes"] = subtask["allowed_modes"]; @@ -674,6 +747,10 @@ export class ULabel { // Track global state this.is_shaking = false; this.annotation_navigation_toast_timeout = null; + // Set true by destroy(); subsequent calls short-circuit. + this.is_destroyed = false; + // MutationObserver used by opt-in auto-teardown; may be null. + this.mutation_observer = null; } init(callback) { @@ -693,6 +770,48 @@ export class ULabel { if (this.get_current_subtask()["state"]["annotation_mode"] === "bitmask") { BrushToolboxItem.show_brush_toolbox_item(); } + + // Install the opt-in auto-teardown observer once the container is in the DOM. + if (this.config?.["auto_destroy_on_detach"]) { + this._install_auto_destroy_observer(); + } + } + + /** + * Watch for the container leaving the document and call `destroy()` when it does. + * Callback holds a WeakRef so this observer alone can't pin the instance in memory, + * and defers the teardown decision by one animation frame so brief detach/reattach + * (portals, jQuery `.detach()`, layout reparenting) does not trigger a false positive. + */ + _install_auto_destroy_observer() { + if (!this.config?.["auto_destroy_on_detach"]) return; + if (typeof MutationObserver === "undefined" || typeof WeakRef === "undefined") return; + const container_id = this.config?.["container_id"]; + if (!container_id) return; + const container = document.getElementById(container_id); + if (container == null || !container.isConnected) return; + + const root = typeof container.getRootNode === "function" ? container.getRootNode() : document; + const weak = new WeakRef(this); + const observer = new MutationObserver(() => { + const self = weak.deref(); + if (!self || self.is_destroyed) return; + const c = document.getElementById(self.config?.["container_id"]); + if (c != null && c.isConnected) return; + requestAnimationFrame(() => { + const s = weak.deref(); + if (!s || s.is_destroyed) return; + const cc = document.getElementById(s.config?.["container_id"]); + if (cc != null && cc.isConnected) return; + try { + s.destroy(); + } catch (err) { + log_message(`auto-teardown destroy() threw: ${err}`, LogLevel.ERROR, true); + } + }); + }); + observer.observe(root, { childList: true, subtree: true }); + this.mutation_observer = observer; } version() { @@ -1318,8 +1437,10 @@ export class ULabel { subtask = this.get_current_subtask_key(); } + const annotation = this.subtasks[subtask]["annotations"]["access"][annotation_id]; + // Remove the annotation_id from the canvas context list - const canvas_id = this.subtasks[subtask]["annotations"]["access"][annotation_id]["canvas_id"]; + const canvas_id = annotation["canvas_id"]; const canvas_context = this.subtasks[subtask]["state"]["annotation_contexts"][canvas_id]; const annotation_ids = canvas_context["annotation_ids"]; const idx = annotation_ids.indexOf(annotation_id); @@ -1335,6 +1456,14 @@ export class ULabel { // Otherwise, redraw the remaining annotations this.redraw_all_annotations_in_annotation_context(canvas_id, subtask); } + + // Release bitmask-only caches so the mask Uint8Array and tinted stencil canvas + // are collectible even if the annotation object outlives this call. + if (annotation["spatial_type"] === "bitmask") { + delete annotation["_mask"]; + delete annotation["_mask_render"]; + delete annotation["_bitmask_box_hint"]; + } } // Get the element id for a nonspatial annotation row @@ -2283,6 +2412,10 @@ export class ULabel { * @param {boolean} nonspatial_only if true, only redraw nonspatial annotations */ redraw_all_annotations(subtask = null, offset = null, nonspatial_only = false) { + if (this.is_destroyed) { + log_message("redraw_all_annotations called on a destroyed ULabel instance", LogLevel.WARNING, true); + return; + } // TODO(3d) if (subtask === null) { for (const st in this.subtasks) { @@ -6937,6 +7070,10 @@ export class ULabel { // Allow for external access and modification of annotations within a subtask get_annotations(subtask) { + if (this.is_destroyed) { + log_message("get_annotations called on a destroyed ULabel instance", LogLevel.WARNING, true); + return []; + } let ret = []; for (let i = 0; i < this.subtasks[subtask]["annotations"]["ordering"].length; i++) { let id = this.subtasks[subtask]["annotations"]["ordering"][i]; @@ -6948,6 +7085,10 @@ export class ULabel { } async set_annotations(new_annotations, subtask) { + if (this.is_destroyed) { + log_message("set_annotations called on a destroyed ULabel instance", LogLevel.WARNING, true); + return; + } // Show the loader while re-initializing annotations, since this is similar to a new init const container = document.getElementById(this.config["container_id"]); ULabelLoader.add_loader_div(container); @@ -6958,7 +7099,7 @@ export class ULabel { // Undo/redo won't work through a get/set this.reset_interaction_state(); this.subtasks[subtask]["actions"]["stream"] = []; - this.subtasks[subtask]["actions"]["undo_stack"] = []; + this.subtasks[subtask]["actions"]["undone_stack"] = []; // Remove canvases for spatial annotations for (let i = 0; i < this.subtasks[subtask]["annotations"]["ordering"].length; i++) { diff --git a/src/listeners.ts b/src/listeners.ts index 4802f13d..dee71fcc 100644 --- a/src/listeners.ts +++ b/src/listeners.ts @@ -747,7 +747,13 @@ export function remove_ulabel_listeners( // Remove jquery event listeners with the ulabel namespace $(document).off(ULABEL_NAMESPACE); $(window).off(ULABEL_NAMESPACE); - $(".id_dialog").off(ULABEL_NAMESPACE); + // Scope id_dialog cleanup to this instance so a sibling ULabel keeps its handlers. + const container_id = ulabel?.config?.container_id; + if (container_id) { + $(`#${container_id} .id_dialog`).off(ULABEL_NAMESPACE); + } else { + $(".id_dialog").off(ULABEL_NAMESPACE); + } // Go through each resize observer and disconnect them if (ulabel.resize_observers != null) { diff --git a/src/version.js b/src/version.js index 1e78b199..5abdaa45 100644 --- a/src/version.js +++ b/src/version.js @@ -1 +1 @@ -export const ULABEL_VERSION = "0.25.1"; +export const ULABEL_VERSION = "0.26.0"; diff --git a/tests/teardown.test.js b/tests/teardown.test.js new file mode 100644 index 00000000..3aff891f --- /dev/null +++ b/tests/teardown.test.js @@ -0,0 +1,194 @@ +// Tests for destroy() and the opt-in auto-teardown observer. +const { ULabel } = require("./testing-utils/build_loader"); +const { ULabelMask } = require("../build/mask_utils"); + +describe("Teardown", () => { + let base_config; + const container_id = "test-container"; + + beforeEach(() => { + document.body.innerHTML = `
`; + base_config = { + container_id: container_id, + image_data: "test-image.png", + username: "test-user", + initial_line_size: 2, + submit_buttons: [{ name: "Submit", hook: jest.fn() }], + subtasks: { + test_task: { + display_name: "Test Task", + classes: [{ name: "TestClass", id: 1, color: "red" }], + allowed_modes: ["bbox", "polygon", "point", "bitmask"], + resume_from: null, + }, + }, + }; + }); + + function build_ulabel_with_bitmask() { + const mask = ULabelMask.create_empty(8, 6); + mask.paint_circle(4, 3, 2, 1); + const config = { + ...base_config, + subtasks: { + test_task: { + ...base_config.subtasks.test_task, + resume_from: [ + { + spatial_type: "bitmask", + spatial_payload: mask.to_rle(), + classification_payloads: [{ class_id: 1, confidence: 1.0 }], + }, + ], + }, + }, + }; + return new ULabel(config); + } + + describe("destroy()", () => { + test("releases bitmask caches on every annotation", () => { + const ulabel = build_ulabel_with_bitmask(); + const anno_id = ulabel.subtasks.test_task.annotations.ordering[0]; + const annotation = ulabel.subtasks.test_task.annotations.access[anno_id]; + + // Force the mask cache to be populated. + ulabel.get_bitmask(annotation); + expect(annotation._mask).toBeDefined(); + + ulabel.destroy(); + + // subtasks were wiped, but we still hold `annotation` from before. + expect(annotation._mask).toBeUndefined(); + expect(annotation._mask_render).toBeUndefined(); + expect(annotation._bitmask_box_hint).toBeUndefined(); + }); + + test("empties subtasks.annotations and action streams", () => { + const ulabel = build_ulabel_with_bitmask(); + // Seed a fake action so we can prove the stream is emptied. + ulabel.subtasks.test_task.actions.stream.push({ act_type: "noop" }); + ulabel.subtasks.test_task.actions.undone_stack.push({ act_type: "noop" }); + + ulabel.destroy(); + + expect(ulabel.subtasks.test_task.annotations.ordering).toEqual([]); + expect(ulabel.subtasks.test_task.annotations.access).toEqual({}); + expect(ulabel.subtasks.test_task.actions.stream).toEqual([]); + expect(ulabel.subtasks.test_task.actions.undone_stack).toEqual([]); + }); + + test("is idempotent", () => { + const ulabel = build_ulabel_with_bitmask(); + ulabel.destroy(); + expect(ulabel.is_destroyed).toBe(true); + // Second call must not throw. + expect(() => ulabel.destroy()).not.toThrow(); + }); + + test("wipes the container DOM", () => { + const ulabel = build_ulabel_with_bitmask(); + const container = document.getElementById(container_id); + container.appendChild(document.createElement("canvas")); + expect(container.children.length).toBeGreaterThan(0); + + ulabel.destroy(); + + expect(container.children.length).toBe(0); + }); + }); + + describe("post-destroy guards", () => { + test("get_annotations returns [] and does not throw", () => { + const ulabel = build_ulabel_with_bitmask(); + ulabel.destroy(); + expect(ulabel.get_annotations("test_task")).toEqual([]); + }); + + test("redraw_all_annotations is a no-op", () => { + const ulabel = build_ulabel_with_bitmask(); + ulabel.destroy(); + expect(() => ulabel.redraw_all_annotations()).not.toThrow(); + }); + + test("set_annotations is a no-op", async () => { + const ulabel = build_ulabel_with_bitmask(); + ulabel.destroy(); + await expect(ulabel.set_annotations([], "test_task")).resolves.toBeUndefined(); + }); + }); + + describe("auto-teardown observer", () => { + function wait_microtask() { + return new Promise((resolve) => setTimeout(resolve, 0)); + } + function wait_frame() { + return new Promise((resolve) => { + if (typeof requestAnimationFrame === "function") { + requestAnimationFrame(() => resolve()); + } else { + setTimeout(resolve, 16); + } + }); + } + + test("does not install an observer when the flag is explicitly off", () => { + const ulabel = build_ulabel_with_bitmask(); + ulabel.config.auto_destroy_on_detach = false; + ulabel._install_auto_destroy_observer(); + expect(ulabel.mutation_observer).toBeNull(); + }); + + test("installs an observer when the flag is on and calls destroy() on container removal", async () => { + const ulabel = build_ulabel_with_bitmask(); + ulabel.config.auto_destroy_on_detach = true; + ulabel._install_auto_destroy_observer(); + expect(ulabel.mutation_observer).not.toBeNull(); + + // Remove the container from the DOM. + document.getElementById(container_id).remove(); + + // The observer callback runs on a microtask, then defers the destroy decision + // by one animation frame. Wait for both. + await wait_microtask(); + await wait_frame(); + await wait_microtask(); + + expect(ulabel.is_destroyed).toBe(true); + }); + + test("does not destroy when the container is reparented within a frame", async () => { + const ulabel = build_ulabel_with_bitmask(); + ulabel.config.auto_destroy_on_detach = true; + ulabel._install_auto_destroy_observer(); + + const container = document.getElementById(container_id); + const holder = document.createElement("div"); + document.body.appendChild(holder); + + // Detach and reattach synchronously; observer fires once but by the time + // its rAF check runs the container is connected again. + container.remove(); + holder.appendChild(container); + + await wait_microtask(); + await wait_frame(); + await wait_microtask(); + + expect(ulabel.is_destroyed).toBe(false); + }); + + test("destroy() disconnects the observer", () => { + const ulabel = build_ulabel_with_bitmask(); + ulabel.config.auto_destroy_on_detach = true; + ulabel._install_auto_destroy_observer(); + const observer = ulabel.mutation_observer; + const disconnect_spy = jest.spyOn(observer, "disconnect"); + + ulabel.destroy(); + + expect(disconnect_spy).toHaveBeenCalled(); + expect(ulabel.mutation_observer).toBeNull(); + }); + }); +}); From 9f9212ce06ba777eef0a5dbacb7e12d2a56bb603 Mon Sep 17 00:00:00 2001 From: TrevorBurgoyne Date: Tue, 11 Aug 2026 17:22:14 -0500 Subject: [PATCH 2/6] apply suggestions from review --- src/index.js | 2 ++ src/listeners.ts | 4 +++- src/toolbox.ts | 3 ++- tests/teardown.test.js | 19 +++++++++++++++++++ 4 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/index.js b/src/index.js index c40fe0fb..1431953e 100644 --- a/src/index.js +++ b/src/index.js @@ -784,6 +784,8 @@ export class ULabel { * (portals, jQuery `.detach()`, layout reparenting) does not trigger a false positive. */ _install_auto_destroy_observer() { + if (this.is_destroyed) return; + if (this.mutation_observer != null) return; if (!this.config?.["auto_destroy_on_detach"]) return; if (typeof MutationObserver === "undefined" || typeof WeakRef === "undefined") return; const container_id = this.config?.["container_id"]; diff --git a/src/listeners.ts b/src/listeners.ts index dee71fcc..469d8500 100644 --- a/src/listeners.ts +++ b/src/listeners.ts @@ -390,7 +390,9 @@ export function create_ulabel_listeners( ulabel: ULabel, ) { // ================= Mouse Events in the ID Dialog ================= - const id_dialog = $(".id_dialog"); + // Scoped to this instance's container so multi-instance pages don't attach + // handlers onto sibling ULabels' dialogs (the teardown side is scoped the same way). + const id_dialog = $(`#${ulabel.config["container_id"]} .id_dialog`); id_dialog.on( "mousemove" + ULABEL_NAMESPACE, (mouse_event) => { diff --git a/src/toolbox.ts b/src/toolbox.ts index a2e5103a..b1253817 100644 --- a/src/toolbox.ts +++ b/src/toolbox.ts @@ -1614,7 +1614,8 @@ export class RecolorActiveItem extends ToolboxItem { // https://typescript-eslint.io/rules/no-this-alias/ // eslint-disable-next-line @typescript-eslint/no-this-alias const that = this; - $(".id_dialog").on("mousemove.ulabel", function (mouse_event) { + // Scoped to this instance's container so we don't attach onto sibling ULabels' dialogs. + $(`#${that.ulabel.config.container_id} .id_dialog`).on("mousemove.ulabel", function (mouse_event) { if (!that.ulabel.subtasks[current_subtask_key].state.idd_thumbnail) { that.ulabel.handle_id_dialog_hover(mouse_event); } diff --git a/tests/teardown.test.js b/tests/teardown.test.js index 3aff891f..e971c98b 100644 --- a/tests/teardown.test.js +++ b/tests/teardown.test.js @@ -190,5 +190,24 @@ describe("Teardown", () => { expect(disconnect_spy).toHaveBeenCalled(); expect(ulabel.mutation_observer).toBeNull(); }); + + test("is idempotent: calling _install_auto_destroy_observer twice keeps a single observer", () => { + const ulabel = build_ulabel_with_bitmask(); + ulabel.config.auto_destroy_on_detach = true; + ulabel._install_auto_destroy_observer(); + const first = ulabel.mutation_observer; + expect(first).not.toBeNull(); + + ulabel._install_auto_destroy_observer(); + expect(ulabel.mutation_observer).toBe(first); + }); + + test("does not install an observer on a destroyed instance", () => { + const ulabel = build_ulabel_with_bitmask(); + ulabel.config.auto_destroy_on_detach = true; + ulabel.destroy(); + ulabel._install_auto_destroy_observer(); + expect(ulabel.mutation_observer).toBeNull(); + }); }); }); From 26feae975e0f4704732824183e3480e806e8b8af Mon Sep 17 00:00:00 2001 From: TrevorBurgoyne Date: Tue, 11 Aug 2026 17:30:33 -0500 Subject: [PATCH 3/6] another round of review --- package-lock.json | 4 ++-- src/index.js | 23 ++++++++++++++-------- tests/teardown.test.js | 44 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 10 deletions(-) diff --git a/package-lock.json b/package-lock.json index 09f3c256..6312a914 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "ulabel", - "version": "0.25.1", + "version": "0.26.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "ulabel", - "version": "0.25.1", + "version": "0.26.0", "license": "MIT", "devDependencies": { "@eslint/config-inspector": "^1.3.0", diff --git a/src/index.js b/src/index.js index 1431953e..88877a86 100644 --- a/src/index.js +++ b/src/index.js @@ -201,11 +201,15 @@ export class ULabel { this.resize_observers = []; // 7. Wipe the container DOM (canvases, id dialogs, brush circle, enders, overlays). - const container_id = this.config?.["container_id"]; - if (container_id) { - const container = document.getElementById(container_id); - if (container) container.innerHTML = ""; + // Prefer the captured owned-container node so a same-id replacement mounted by an SPA + // between detach and this call is left untouched. + let container = this._owned_container; + if (container == null) { + const container_id = this.config?.["container_id"]; + if (container_id) container = document.getElementById(container_id); } + if (container) container.innerHTML = ""; + this._owned_container = null; this.is_init = false; this.is_destroyed = true; @@ -751,6 +755,9 @@ export class ULabel { this.is_destroyed = false; // MutationObserver used by opt-in auto-teardown; may be null. this.mutation_observer = null; + // The specific container HTMLElement this instance owns. Captured at observer-install + // time so an SPA replacement using the same id can't be mistaken for our container. + this._owned_container = null; } init(callback) { @@ -793,18 +800,18 @@ export class ULabel { const container = document.getElementById(container_id); if (container == null || !container.isConnected) return; + // Own this exact node from now on: node identity, not id equality. + this._owned_container = container; const root = typeof container.getRootNode === "function" ? container.getRootNode() : document; const weak = new WeakRef(this); const observer = new MutationObserver(() => { const self = weak.deref(); if (!self || self.is_destroyed) return; - const c = document.getElementById(self.config?.["container_id"]); - if (c != null && c.isConnected) return; + if (container.isConnected) return; requestAnimationFrame(() => { const s = weak.deref(); if (!s || s.is_destroyed) return; - const cc = document.getElementById(s.config?.["container_id"]); - if (cc != null && cc.isConnected) return; + if (container.isConnected) return; try { s.destroy(); } catch (err) { diff --git a/tests/teardown.test.js b/tests/teardown.test.js index e971c98b..54b660fe 100644 --- a/tests/teardown.test.js +++ b/tests/teardown.test.js @@ -209,5 +209,49 @@ describe("Teardown", () => { ulabel._install_auto_destroy_observer(); expect(ulabel.mutation_observer).toBeNull(); }); + + test("still destroys when the container is replaced with a same-id node", async () => { + const ulabel = build_ulabel_with_bitmask(); + ulabel.config.auto_destroy_on_detach = true; + ulabel._install_auto_destroy_observer(); + + // SPA-style swap: remove ours, mount a fresh element with the same id. + const original = document.getElementById(container_id); + original.remove(); + const replacement = document.createElement("div"); + replacement.id = container_id; + const sentinel = document.createElement("span"); + sentinel.textContent = "replacement content"; + replacement.appendChild(sentinel); + document.body.appendChild(replacement); + + await wait_microtask(); + await wait_frame(); + await wait_microtask(); + + // Old instance was torn down (our node isn't connected any more). + expect(ulabel.is_destroyed).toBe(true); + // The replacement's DOM was NOT nuked by our destroy path. + expect(document.getElementById(container_id)).toBe(replacement); + expect(replacement.contains(sentinel)).toBe(true); + }); + + test("manual destroy() does not clear a same-id replacement", () => { + const ulabel = build_ulabel_with_bitmask(); + ulabel.config.auto_destroy_on_detach = true; + ulabel._install_auto_destroy_observer(); + + const original = document.getElementById(container_id); + original.remove(); + const replacement = document.createElement("div"); + replacement.id = container_id; + const sentinel = document.createElement("span"); + replacement.appendChild(sentinel); + document.body.appendChild(replacement); + + ulabel.destroy(); + + expect(replacement.contains(sentinel)).toBe(true); + }); }); }); From 0ff3dd9a0ef981be8720bab44881121b844800b5 Mon Sep 17 00:00:00 2001 From: TrevorBurgoyne Date: Wed, 12 Aug 2026 09:42:45 -0500 Subject: [PATCH 4/6] more review --- CHANGELOG.md | 1 - api_spec.md | 2 ++ index.d.ts | 6 ++++++ src/index.js | 37 ++++++++++++++++++++++++++----------- src/initializer.ts | 8 ++++++++ src/listeners.ts | 12 ++---------- src/toolbox.ts | 3 +-- tests/teardown.test.js | 19 +++++++++++++++++++ 8 files changed, 64 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d929e27..34988a59 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,6 @@ All notable changes to this project will be documented here. - New `auto_destroy_on_detach` config option (default `true`). When enabled, ULabel installs a `MutationObserver` on the container's root and calls `destroy()` automatically after the container leaves the DOM. - `set_annotations()`, `get_annotations()`, and `redraw_all_annotations()` now short-circuit with a warning if called after `destroy()`. - Fixed a latent typo in `set_annotations()` that wrote to a non-existent `undo_stack` property instead of clearing the real `undone_stack`. - - `remove_ulabel_listeners()` now scopes its `.id_dialog` cleanup to the instance's container, so tearing down one ULabel no longer strips id-dialog handlers from siblings on the same page. ## [0.25.1] - Aug 10th, 2026 diff --git a/api_spec.md b/api_spec.md index 7308ef3f..7e9493c9 100644 --- a/api_spec.md +++ b/api_spec.md @@ -32,6 +32,8 @@ ulabel.init(() => {/* behavior on ready */}) `ULabel` is the only name that `ulabel.js` will add to the global namespace. +> ULabel is designed for **a single instance per page**. Toolbox handlers, id dialogs, and global keybinds bind to shared DOM ids and delegated selectors on `document`, so mounting more than one `ULabel` simultaneously is not supported. + The constructor is used to specify the configuration for an "annotation session". It has the following interface ```javascript diff --git a/index.d.ts b/index.d.ts index ba2f8950..be166584 100644 --- a/index.d.ts +++ b/index.d.ts @@ -300,6 +300,7 @@ export type ULabelConstructorArgs = { initial_line_size?: number; instructions_url?: string; toolbox_order?: AllowedToolboxItem[]; + auto_destroy_on_detach?: boolean; /** @deprecated Use top-level properties instead. */ config_data?: object; }; @@ -443,6 +444,11 @@ export class ULabel { public destroy(): void; // True after destroy() has run; subsequent destroy() calls are no-ops. is_destroyed: boolean; + // The container HTMLElement this instance owns, captured at init. Used by destroy() and + // the auto-teardown observer to avoid mistaking a same-id replacement for our container. + _owned_container: HTMLElement | null; + // Install the auto-teardown MutationObserver; called by init and idempotent. + _install_auto_destroy_observer(): void; // Static functions static version(): string; diff --git a/src/index.js b/src/index.js index 88877a86..d4b2b10c 100644 --- a/src/index.js +++ b/src/index.js @@ -777,11 +777,6 @@ export class ULabel { if (this.get_current_subtask()["state"]["annotation_mode"] === "bitmask") { BrushToolboxItem.show_brush_toolbox_item(); } - - // Install the opt-in auto-teardown observer once the container is in the DOM. - if (this.config?.["auto_destroy_on_detach"]) { - this._install_auto_destroy_observer(); - } } /** @@ -795,13 +790,17 @@ export class ULabel { if (this.mutation_observer != null) return; if (!this.config?.["auto_destroy_on_detach"]) return; if (typeof MutationObserver === "undefined" || typeof WeakRef === "undefined") return; - const container_id = this.config?.["container_id"]; - if (!container_id) return; - const container = document.getElementById(container_id); - if (container == null || !container.isConnected) return; + // _owned_container is captured by ulabel_init(); fall back to id lookup for direct callers. + let container = this._owned_container; + if (container == null) { + const container_id = this.config?.["container_id"]; + if (!container_id) return; + container = document.getElementById(container_id); + if (container == null) return; + this._owned_container = container; + } + if (!container.isConnected) return; - // Own this exact node from now on: node identity, not id equality. - this._owned_container = container; const root = typeof container.getRootNode === "function" ? container.getRootNode() : document; const weak = new WeakRef(this); const observer = new MutationObserver(() => { @@ -6994,6 +6993,10 @@ export class ULabel { } async swap_frame_image(new_src, frame = 0) { + if (this.is_destroyed) { + log_message("swap_frame_image called on a destroyed ULabel instance", LogLevel.WARNING, true); + return null; + } const img = $(`img#${this.config["image_id_pfx"]}__${frame}`); const ret = img.attr("src"); @@ -7003,6 +7006,12 @@ export class ULabel { // Yield so the browser can paint the loader before swapping the image await ULabelLoader.wait_for_render(); + // Recheck: destroy() may have run during the paint yield. + if (this.is_destroyed) { + log_message("swap_frame_image aborted; ULabel was destroyed during load", LogLevel.WARNING, true); + return null; + } + try { img.attr("src", new_src); // Wait for the new image to be decoded and ready to display @@ -7104,6 +7113,12 @@ export class ULabel { // Yield so the browser can paint the loader before the heavy synchronous work below await ULabelLoader.wait_for_render(); + // Recheck: destroy() (manual or auto) may have run during the paint yield. + if (this.is_destroyed) { + log_message("set_annotations aborted; ULabel was destroyed during load", LogLevel.WARNING, true); + return; + } + try { // Undo/redo won't work through a get/set this.reset_interaction_state(); diff --git a/src/initializer.ts b/src/initializer.ts index 4ccc4844..61093ff8 100644 --- a/src/initializer.ts +++ b/src/initializer.ts @@ -159,6 +159,10 @@ export async function ulabel_init( ulabel.config.toolbox_order, ); + // Own this specific node from now on so destroy() and the auto-teardown observer never + // mistake a same-id replacement mounted by an SPA for our container. + ulabel._owned_container = document.getElementById(ulabel.config["container_id"]); + // Detect night cookie if (NightModeCookie.exists_in_document()) { $("#" + ulabel.config["container_id"]).addClass("ulabel-night"); @@ -235,6 +239,10 @@ export async function ulabel_init( ULabelLoader.remove_loader_div(); + // Arm auto-teardown before the user callback so a callback that synchronously removes + // the container still triggers destroy(). No-op when the flag is disabled. + ulabel._install_auto_destroy_observer(); + // Call the user-provided callback user_callback(); ulabel.after_init(); diff --git a/src/listeners.ts b/src/listeners.ts index 469d8500..4802f13d 100644 --- a/src/listeners.ts +++ b/src/listeners.ts @@ -390,9 +390,7 @@ export function create_ulabel_listeners( ulabel: ULabel, ) { // ================= Mouse Events in the ID Dialog ================= - // Scoped to this instance's container so multi-instance pages don't attach - // handlers onto sibling ULabels' dialogs (the teardown side is scoped the same way). - const id_dialog = $(`#${ulabel.config["container_id"]} .id_dialog`); + const id_dialog = $(".id_dialog"); id_dialog.on( "mousemove" + ULABEL_NAMESPACE, (mouse_event) => { @@ -749,13 +747,7 @@ export function remove_ulabel_listeners( // Remove jquery event listeners with the ulabel namespace $(document).off(ULABEL_NAMESPACE); $(window).off(ULABEL_NAMESPACE); - // Scope id_dialog cleanup to this instance so a sibling ULabel keeps its handlers. - const container_id = ulabel?.config?.container_id; - if (container_id) { - $(`#${container_id} .id_dialog`).off(ULABEL_NAMESPACE); - } else { - $(".id_dialog").off(ULABEL_NAMESPACE); - } + $(".id_dialog").off(ULABEL_NAMESPACE); // Go through each resize observer and disconnect them if (ulabel.resize_observers != null) { diff --git a/src/toolbox.ts b/src/toolbox.ts index b1253817..a2e5103a 100644 --- a/src/toolbox.ts +++ b/src/toolbox.ts @@ -1614,8 +1614,7 @@ export class RecolorActiveItem extends ToolboxItem { // https://typescript-eslint.io/rules/no-this-alias/ // eslint-disable-next-line @typescript-eslint/no-this-alias const that = this; - // Scoped to this instance's container so we don't attach onto sibling ULabels' dialogs. - $(`#${that.ulabel.config.container_id} .id_dialog`).on("mousemove.ulabel", function (mouse_event) { + $(".id_dialog").on("mousemove.ulabel", function (mouse_event) { if (!that.ulabel.subtasks[current_subtask_key].state.idd_thumbnail) { that.ulabel.handle_id_dialog_hover(mouse_event); } diff --git a/tests/teardown.test.js b/tests/teardown.test.js index 54b660fe..8f0c2010 100644 --- a/tests/teardown.test.js +++ b/tests/teardown.test.js @@ -253,5 +253,24 @@ describe("Teardown", () => { expect(replacement.contains(sentinel)).toBe(true); }); + + test("manual destroy() (auto flag off) also spares a same-id replacement", () => { + const ulabel = build_ulabel_with_bitmask(); + ulabel.config.auto_destroy_on_detach = false; + // Simulate what ulabel_init does: capture the owned container node before use. + ulabel._owned_container = document.getElementById(container_id); + + const original = ulabel._owned_container; + original.remove(); + const replacement = document.createElement("div"); + replacement.id = container_id; + const sentinel = document.createElement("span"); + replacement.appendChild(sentinel); + document.body.appendChild(replacement); + + ulabel.destroy(); + + expect(replacement.contains(sentinel)).toBe(true); + }); }); }); From 2e82b4c242d38812a070ae19a693759fdb503e2f Mon Sep 17 00:00:00 2001 From: TrevorBurgoyne Date: Wed, 12 Aug 2026 10:13:48 -0500 Subject: [PATCH 5/6] another --- api_spec.md | 2 ++ src/index.js | 9 +++++++++ src/initializer.ts | 11 +++++++---- tests/teardown.test.js | 26 ++++++++++++++++++++++++++ 4 files changed, 44 insertions(+), 4 deletions(-) diff --git a/api_spec.md b/api_spec.md index 7e9493c9..08bb3b66 100644 --- a/api_spec.md +++ b/api_spec.md @@ -621,6 +621,8 @@ When `false`, new annotations will be limited to points within the image, and at ### `auto_destroy_on_detach` When `true` (the default), ULabel installs a `MutationObserver` on the container's root and calls [`destroy()`](#destroy) automatically after the container is removed from the DOM. The observer holds the ULabel instance through a `WeakRef` (so it cannot pin the instance in memory on its own) and defers the teardown decision by one animation frame so brief detach/reattach cycles (portals, jQuery `.detach()`, layout reparenting) do not trigger a false-positive teardown. Set to `false` to opt out and manage teardown manually via [`destroy()`](#destroy). +> **Same-id replacement caveat.** With the default `true`, the one-frame grace period means a caller who removes the old container and mounts a new `
` with the same `container_id` *within the same animation frame* can briefly have two `ULabel` instances attached to `document`; when the old instance's teardown runs it will remove `.ulabel`-namespaced document/window handlers belonging to the new instance too. If your SPA does synchronous same-id replacement, set `auto_destroy_on_detach: false` and call `oldUlabel.destroy()` yourself *before* mounting the replacement — `destroy()` is synchronous, so this ordering is race-free. + ## Display Utility Functions diff --git a/src/index.js b/src/index.js index d4b2b10c..b4f6aa98 100644 --- a/src/index.js +++ b/src/index.js @@ -166,7 +166,14 @@ export class ULabel { // In-progress bitmask move snapshot canvas (skipped by end_bitmask_move on interrupt). if (this.state) { this.state["bitmask_move_overlay"] = null; + // Any queued rAF from schedule_continue_move()/edit paths checks is_in_move on + // the current subtask; reset_interaction_state() flips those flags so those + // callbacks become no-ops when they fire. + this.state["last_move"] = null; } + try { + this.reset_interaction_state(); + } catch { /* subtasks may be partially torn down; ignore */ } // 4. Drop bitmask caches on every annotation, in every subtask, and clear the // action streams so retained RLE payloads (before/after) are collectible. @@ -768,6 +775,8 @@ export class ULabel { * Code to be called after ULabel has finished initializing. */ after_init() { + // A user_callback (called just before this) may have invoked destroy(). + if (this.is_destroyed) return; // Perform the after_init method for each toolbox item for (const toolbox_item of this.toolbox.items) { toolbox_item.after_init(); diff --git a/src/initializer.ts b/src/initializer.ts index 61093ff8..3ccde13c 100644 --- a/src/initializer.ts +++ b/src/initializer.ts @@ -163,6 +163,10 @@ export async function ulabel_init( // mistake a same-id replacement mounted by an SPA for our container. ulabel._owned_container = document.getElementById(ulabel.config["container_id"]); + // Arm auto-teardown before the image decode await so a container removal during decode + // still triggers destroy() rather than silently leaving init running on a detached tree. + ulabel._install_auto_destroy_observer(); + // Detect night cookie if (NightModeCookie.exists_in_document()) { $("#" + ulabel.config["container_id"]).addClass("ulabel-night"); @@ -170,6 +174,9 @@ export async function ulabel_init( const first_bg_img = document.getElementById(`${ulabel.config["image_id_pfx"]}__0`); await first_bg_img.decode(); + // Container may have been removed (and destroy() may have run) during decode. + if (ulabel.is_destroyed) return; + make_image_canvases(ulabel, first_bg_img); // Once the image dimensions are known, we can resize annotations if needed @@ -239,10 +246,6 @@ export async function ulabel_init( ULabelLoader.remove_loader_div(); - // Arm auto-teardown before the user callback so a callback that synchronously removes - // the container still triggers destroy(). No-op when the flag is disabled. - ulabel._install_auto_destroy_observer(); - // Call the user-provided callback user_callback(); ulabel.after_init(); diff --git a/tests/teardown.test.js b/tests/teardown.test.js index 8f0c2010..44e7932b 100644 --- a/tests/teardown.test.js +++ b/tests/teardown.test.js @@ -116,6 +116,32 @@ describe("Teardown", () => { ulabel.destroy(); await expect(ulabel.set_annotations([], "test_task")).resolves.toBeUndefined(); }); + + test("destroy() clears interaction flags so pending rAF paths become no-ops", () => { + const ulabel = build_ulabel_with_bitmask(); + // Simulate an in-progress move: schedule_continue_move sets is_in_move and last_move. + const subtask = ulabel.subtasks.test_task; + subtask.state.is_in_move = true; + subtask.state.is_in_edit = true; + subtask.state.is_in_progress = true; + subtask.state.active_id = "fake_id"; + ulabel.state.last_move = { clientX: 0, clientY: 0 }; + + ulabel.destroy(); + + expect(subtask.state.is_in_move).toBe(false); + expect(subtask.state.is_in_edit).toBe(false); + expect(subtask.state.is_in_progress).toBe(false); + expect(subtask.state.active_id).toBeNull(); + expect(ulabel.state.last_move).toBeNull(); + }); + + test("after_init() on a destroyed instance is a no-op", () => { + const ulabel = build_ulabel_with_bitmask(); + ulabel.destroy(); + // toolbox is null after destroy; unguarded after_init() would throw. + expect(() => ulabel.after_init()).not.toThrow(); + }); }); describe("auto-teardown observer", () => { From 901f2ba2e8433ad82f8b6e8fab634ba92d55a0e5 Mon Sep 17 00:00:00 2001 From: TrevorBurgoyne Date: Wed, 12 Aug 2026 10:35:12 -0500 Subject: [PATCH 6/6] clean canvases --- src/index.js | 9 +++++++++ tests/teardown.test.js | 17 +++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/src/index.js b/src/index.js index b4f6aa98..578c58dd 100644 --- a/src/index.js +++ b/src/index.js @@ -191,8 +191,17 @@ export class ULabel { } if (subtask.state) { subtask.state["annotation_contexts"] = {}; + // Front/back contexts each retain an image-sized canvas backing store; letting + // them go lets `container.innerHTML = ""` below actually release those pixels. + subtask.state["back_context"] = null; + subtask.state["front_context"] = null; + // A stroke interrupted before finish_bitmask() leaves a full pre-stroke RLE here. + subtask.state["bitmask_stroke"] = null; } } + if (this.state) { + this.state["last_brush_stroke"] = null; + } // 5. Break the toolbox <-> ulabel back-reference. Toolbox items keep a // `this.ulabel` and are stored on `this.toolbox.items`. diff --git a/tests/teardown.test.js b/tests/teardown.test.js index 44e7932b..65d33f74 100644 --- a/tests/teardown.test.js +++ b/tests/teardown.test.js @@ -142,6 +142,23 @@ describe("Teardown", () => { // toolbox is null after destroy; unguarded after_init() would throw. expect(() => ulabel.after_init()).not.toThrow(); }); + + test("releases per-subtask canvas contexts and in-progress bitmask stroke state", () => { + const ulabel = build_ulabel_with_bitmask(); + const subtask = ulabel.subtasks.test_task; + // Simulate a finished init + an interrupted bitmask stroke. + subtask.state.back_context = { canvas: document.createElement("canvas") }; + subtask.state.front_context = { canvas: document.createElement("canvas") }; + subtask.state.bitmask_stroke = { annotation_id: "id", before_rle: { counts: [1, 2, 3], size: [4, 4] } }; + ulabel.state.last_brush_stroke = [10, 20]; + + ulabel.destroy(); + + expect(subtask.state.back_context).toBeNull(); + expect(subtask.state.front_context).toBeNull(); + expect(subtask.state.bitmask_stroke).toBeNull(); + expect(ulabel.state.last_brush_stroke).toBeNull(); + }); }); describe("auto-teardown observer", () => {