[web] Cleanup PDFjs instances in visual editor (#33022)

GitOrigin-RevId: 2aa9ab01f88196fb56dc41749977ca33295c964f

services/web/frontend/js/features/source-editor/extensions/visual/utils/pdf-destroy-lock.ts

@@ -0,0 +1,26 @@
+/**
+ * Coordinates PDF.js document destroys across GraphicsWidget instances.
+ *
+ * PDF.js uses a single shared worker (GlobalWorkerOptions.workerPort) and
+ * rejects getDocument while any previous PDFDocumentLoadingTask.destroy()
+ * are in flight.
+ */
+export class PdfDestroyLock {
+  private readonly pendingDestroys = new Set<Promise<unknown>>()
+
+  /** Resolves once no pdf.destroy() is in flight. */
+  async waitForPending(): Promise<void> {
+    // Loop in case a new destroy is scheduled while we await existing ones.
+    while (this.pendingDestroys.size > 0) {
+      await Promise.all(this.pendingDestroys)
+    }
+  }
+
+  /** Registers an in-flight destroy so waitForPending() will wait for it. */
+  schedule<T>(destroyFn: () => Promise<T>): Promise<T> {
+    const task = destroyFn()
+    this.pendingDestroys.add(task)
+    task.finally(() => this.pendingDestroys.delete(task))
+    return task
+  }
+}

services/web/frontend/js/features/source-editor/extensions/visual/visual-widgets/graphics.ts

@@ -4,10 +4,24 @@ import { isEqual } from 'lodash'
 import { FigureData } from '../../figure-modal'
 import { debugConsole } from '@/utils/debugging'
 import { PreviewPath } from '../../../../../../../types/preview-path'
+import type { PDFDocumentProxy } from 'pdfjs-dist/types/src/display/api'
+import { PdfDestroyLock } from '../utils/pdf-destroy-lock'
+
+// Module level to synchronize across all GraphicsWidgets
+const pdfDestroyLock = new PdfDestroyLock()
+
+function schedulePdfDestroy(pdf: PDFDocumentProxy) {
+  pdfDestroyLock.schedule(() =>
+    pdf.destroy().catch(() => {
+      debugConsole.warn('Failed to destroy PDFjs instance')
+    })
+  )
+}
 
 export class GraphicsWidget extends WidgetType {
   destroyed = false
   height = 300 // for estimatedHeight, updated when the image is loaded
+  pdfInstance: PDFDocumentProxy | null = null
 
   constructor(
     public filePath: string,
@@ -54,6 +68,10 @@ export class GraphicsWidget extends WidgetType {
     ) {
       return true
     }
+    if (this.pdfInstance) {
+      schedulePdfDestroy(this.pdfInstance)
+      this.pdfInstance = null
+    }
     this.renderGraphic(element, view)
     view.requestMeasure()
     return true
@@ -72,6 +90,10 @@ export class GraphicsWidget extends WidgetType {
 
   destroy() {
     this.destroyed = true
+    if (this.pdfInstance) {
+      schedulePdfDestroy(this.pdfInstance)
+      this.pdfInstance = null
+    }
   }
 
   coordsAt(element: HTMLElement) {
@@ -104,7 +126,11 @@ export class GraphicsWidget extends WidgetType {
         {
           const canvas = document.createElement('canvas')
           canvas.classList.add('ol-cm-graphics')
-          this.renderPDF(view, canvas, preview.url).catch(debugConsole.error)
+          this.renderPDF(view, canvas, preview.url).catch(err => {
+            if (!this.destroyed) {
+              debugConsole.error('Failed to render PDF graphics widget', err)
+            }
+          })
           element.append(canvas)
         }
         break
@@ -249,10 +275,23 @@ export class GraphicsWidget extends WidgetType {
       return
     }
 
+    await pdfDestroyLock.waitForPending()
+    if (this.destroyed) {
+      return
+    }
+
     const pdf = await loadPdfDocumentFromUrl(url).promise
-    const page = await pdf.getPage(1)
+    this.pdfInstance = pdf
 
     // bail out if loading the PDF took too long
+    if (this.destroyed) {
+      schedulePdfDestroy(pdf)
+      this.pdfInstance = null
+      return
+    }
+
+    const page = await pdf.getPage(1)
+
     if (this.destroyed) {
       return
     }

services/web/test/frontend/features/source-editor/extensions/pdf-destroy-lock.test.ts

@@ -0,0 +1,167 @@
+import { PdfDestroyLock } from '@/features/source-editor/extensions/visual/utils/pdf-destroy-lock'
+import { expect } from 'chai'
+
+type Deferred<T> = {
+  promise: Promise<T>
+  resolve: (value: T) => void
+  reject: (err: unknown) => void
+}
+
+function defer<T>(): Deferred<T> {
+  let resolveRet!: (value: T) => void
+  let rejectRet!: (err: unknown) => void
+  const promise = new Promise<T>((resolve, reject) => {
+    resolveRet = resolve
+    rejectRet = reject
+  })
+  return { promise, resolve: resolveRet, reject: rejectRet }
+}
+
+// Flush microtasks so that .finally() handlers attached inside schedule()
+// run before we make assertions.
+const flush = () => new Promise(resolve => setTimeout(resolve, 0))
+
+describe('PdfDestroyLock', function () {
+  describe('waitForPending', function () {
+    it('resolves immediately when no destroys are in flight', async function () {
+      const lock = new PdfDestroyLock()
+      let resolved = false
+      await lock.waitForPending().then(() => {
+        resolved = true
+      })
+      expect(resolved).to.be.true
+    })
+
+    it('waits for a pending destroy to settle', async function () {
+      const lock = new PdfDestroyLock()
+      const deferred = defer<void>()
+      lock.schedule(() => deferred.promise)
+
+      let waited = false
+      const wait = lock.waitForPending().then(() => {
+        waited = true
+      })
+
+      await flush()
+      expect(waited).to.be.false
+
+      deferred.resolve()
+      await wait
+      expect(waited).to.be.true
+    })
+
+    it('waits for a destroy that rejects', async function () {
+      const lock = new PdfDestroyLock()
+      const deferred = defer<void>()
+      // Swallow the rejection at the schedule() call site to mirror real usage,
+      // where callers catch errors from destroy().
+      lock.schedule(() => deferred.promise.catch(() => {}))
+
+      let waited = false
+      const wait = lock.waitForPending().then(() => {
+        waited = true
+      })
+
+      await flush()
+      expect(waited).to.be.false
+
+      deferred.reject(new Error('boom'))
+      await wait
+      expect(waited).to.be.true
+    })
+
+    it('waits for destroys scheduled while waiting', async function () {
+      const lock = new PdfDestroyLock()
+      const first = defer<void>()
+      const second = defer<void>()
+
+      lock.schedule(() => first.promise)
+
+      let waited = false
+      const wait = lock.waitForPending().then(() => {
+        waited = true
+      })
+
+      // Schedule a second destroy before the first completes - waitForPending
+      // must loop and wait for this one too.
+      lock.schedule(() => second.promise)
+
+      first.resolve()
+      await flush()
+      expect(waited).to.be.false
+
+      second.resolve()
+      await wait
+      expect(waited).to.be.true
+    })
+
+    it('waits for multiple concurrent destroys', async function () {
+      const lock = new PdfDestroyLock()
+      const first = defer<void>()
+      const second = defer<void>()
+
+      lock.schedule(() => first.promise)
+      lock.schedule(() => second.promise)
+
+      let waited = false
+      const wait = lock.waitForPending().then(() => {
+        waited = true
+      })
+
+      first.resolve()
+      await flush()
+      expect(waited).to.be.false
+
+      second.resolve()
+      await wait
+      expect(waited).to.be.true
+    })
+  })
+
+  describe('schedule', function () {
+    it('invokes the destroy function immediately', function () {
+      const lock = new PdfDestroyLock()
+      let called = false
+      lock.schedule(async () => {
+        called = true
+      })
+      expect(called).to.be.true
+    })
+
+    it('returns the promise from the destroy function', async function () {
+      const lock = new PdfDestroyLock()
+      const result = await lock.schedule(async () => 'done')
+      expect(result).to.equal('done')
+    })
+
+    it('releases the lock after a successful destroy', async function () {
+      const lock = new PdfDestroyLock()
+      await lock.schedule(async () => {})
+      await flush()
+
+      // waitForPending should now resolve synchronously - if the task was not
+      // removed from the set, this would hang.
+      let resolved = false
+      await lock.waitForPending().then(() => {
+        resolved = true
+      })
+      expect(resolved).to.be.true
+    })
+
+    it('releases the lock after a destroy that rejects', async function () {
+      const lock = new PdfDestroyLock()
+      // Mirror real usage: destroyFn swallows its own errors so the internal
+      // .finally() chain in schedule() doesn't leak an unhandled rejection.
+      await lock.schedule(() =>
+        Promise.reject(new Error('boom')).catch(() => {})
+      )
+      await flush()
+
+      let resolved = false
+      await lock.waitForPending().then(() => {
+        resolved = true
+      })
+      expect(resolved).to.be.true
+    })
+  })
+})