Skip to content

fix: the DAM upload memory leaks, backported from feat/list-editors - #219

Merged
volarname merged 5 commits into
mainfrom
fix/memory-leaks-from-main
Sep 29, 2026
Merged

volarname merged 5 commits into
mainfrom
fix/memory-leaks-from-main

Conversation

@volarname

Copy link
Copy Markdown
Contributor

Backports the DAM upload memory leak fixes from feat/list-editors (2.0.0) into 1.x.

Important

Merge with Create a merge commit, not squash or rebase. The branch merges the original commits so their SHAs become shared history with feat/list-editors. Squashing drops that, and the later main → feat/list-editors merge would conflict on every file below.

These are the first 4 commits of feat/list-editors. Their base (9d337772) is already in main, so nothing from 2.0 comes with them:

  • 6b0ce75f fix(damNotifications): every initDamNotifications() call opened a new WebSocket that nothing could close, so an admin re-running its setup (e.g. admin-dam on each /login / /unauthorized bail-out) piled up live sockets, each re-emitting every message. The connection is now an app-wide singleton, and destroyDamNotifications() is added.
  • cc23d2fa fix(damImageApi): the chunk upload timeout goes in the request config (420 s) instead of being dropped, so chunks no longer run on the client default of 15–30 s.
  • 378409cb chore: unplugin-dts ^1.0.3 → ^1.1.0 (devDependency, declaration build only).
  • 00381b8b fix(uploadQueue): removeByIndex() / clearQueue() clear the fallback timer, so a removed item stops polling fetchAsset and holding its File for up to 550 s. The image widgets settle their collab lock waits on unmount.

Left out on purpose: b6a1333e, e229d012 and 40b6ff73. They are not memory fixes, or their leak doesn't occur in how the admins use them.

The version stays as it is; 1.49.0 comes from #218. The two PRs merge cleanly with each other.

Checks

  • vitest 57 files / 698 tests, vue-tsc, oxlint, eslint, stylelint, lib build: pass.
  • Built .d.ts compared with the published 1.47.0-beta.dev-1784059879-fix1: additions only. That means destroyDamNotifications, plus closeConnection / dispose / status on the initDamNotifications() return. labs.d.ts is identical.
  • Simulated merge into feat/list-editors: the only conflict is the package.json version line.
  • Two independent reviews of this exact branch: GO, no blockers.

Test (one upload)

  1. In the admin, open DevTools → Network → WS: exactly one DAM notifications socket, still one after clicking through a few routes.
  2. Drop an image on an image widget: the item reaches "Uploaded".
  3. Start another upload and close the dialog while it is "Processing": no further GET …/asset/{id} requests.

Notes

  • destroyDamNotifications is exported here but not from 2.0's src/lib.ts; 2.0 should either re-export it or list it as removed.

🤖 Generated with Claude Code

volar added 5 commits September 1, 2026 17:03
initDamNotifications() was an unguarded factory: every call built a fresh useWebSocket state,
and since `close` was discarded and `autoClose: false` disables both of VueUse's auto-cleanups,
nothing could ever close the previous socket. A consumer calling it from a router guard
accumulated one live socket per attempt, each re-emitting every server message into the shared
event bus.

The handle is now memoized by `enabled|webSocketUrl`, gets a `dispose()`, and
destroyDamNotifications() tears it down and invalidates the previous handle. `autoConnect` is
off because the url is a plain string and VueUse's watch would never fire; openConnection()
also guards on socket status, since VueUse's open() closes and re-inits even a live socket.
imageUploadChunk called `client(CHUNK_UPLOAD_TIMEOUT)`, but every shipped factory is declared
`() => AxiosInstance`, so the argument was dropped and chunk requests ran on the instance
default (15s in ugc/cms, 30s in dam). Had a factory honoured it, the raw 420 would have been
read as 420ms and aborted every chunk almost immediately.

The timeout now travels in the per-request config as `CHUNK_UPLOAD_TIMEOUT * 1000`, where
axios' mergeConfig gives it precedence over the instance default.
removeByIndex() and clearQueue() left notificationFallbackTimer running. The callback
reschedules itself for up to 550s, so a removed item kept polling fetchAsset and holding its
File. Seven other paths in the store already cleared it.

The image widgets waited for the collab field lock by recursing through a 100ms setTimeout,
50 times, with nothing tracking it. A late success ran addByFiles — an upload — into an
unmounted widget, and a late failure showed an error with no context. Waits are now tracked
and settled on unmount, and onDrop checks liveness after the await and before showError.
@volarname
volarname merged commit 1c0e907 into main Sep 29, 2026
4 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.

1 participant