From a0dae7fa4fe71d490f0a332efea515e2089692f9 Mon Sep 17 00:00:00 2001 From: onetwothr1 Date: Tue, 28 Jul 2026 16:21:36 +0900 Subject: [PATCH 1/4] =?UTF-8?q?fix:=20=EB=AC=B8=EC=84=9C=20=EA=B5=90?= =?UTF-8?q?=EC=B2=B4=20=EC=8B=9C=20=EC=9D=B4=EC=A0=84=20PDFDocumentProxy?= =?UTF-8?q?=EB=A5=BC=20destroy?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PdfHost.#setDocument가 #doc을 덮어쓸 뿐이라 pdf.js가 워커 자원·페이지 프록시를 계속 붙들었다 — 한 세션에서 문서를 N번 열면 N개가 상주한다. 벤더링본 v2.14.0은 크롭을 살아 있는 캔버스로 유지하므로, Chrome이 메모리 압력에서 캔버스 백킹 스토어를 회수하는 조건(엔진 백로그 B7)에 직접 기여했다. - #setDocument: 뷰어·linkService를 새 문서로 전환한 **뒤** 이전 문서 destroy. 순서가 반대면 뷰어의 동기 teardown이 방금 파괴된 proxy를 만진다 - #releaseDocument: 정리 실패는 console.warn으로 삼켜 새 로드를 막지 않는다 - #awaitLoad: 로드 실패 시 loadingTask.destroy(). pdf.js는 실패 시 promise만 reject하고 전용 워커를 회수하지 않아 실패 N번이 워커 N개로 쌓였다 - getOutlineItems: getOutline 거절을 삼키고 문서 교체를 감지해 빈 목록 반환. 안 막으면 destroy가 유발한 거절이 **이전 로드의 catch**로 흘러가 방금 열린 새 문서 위에 오류 화면을 띄운다 (남은 stale-load 문제는 #37) - #resolveDestPage가 doc을 명시로 받는다 — 교체 후 옛 dest를 새 문서에 풀지 않는다 - downloadCurrentPdf·jumpToOutline: destroy가 노출시키는 거절을 삼킨다 - workerPort를 쓰면 안 되는 이유를 주석으로 고정 (공유 워커면 cross-document kill) Closes #35 typecheck 0 · vitest 35/35 (7 files) · build 0. pdf-host.test.ts는 이 repo 첫 vi.mock 사용이며, src를 되돌리면 6개 중 4개가 실패함을 확인했다. Co-Authored-By: Claude Opus 5 --- docs/fig-extract-integration.md | 27 ++++- src/viewer/main.ts | 12 ++- src/viewer/pdf-host.ts | 71 +++++++++++-- test/pdf-host.test.ts | 177 ++++++++++++++++++++++++++++++++ 4 files changed, 271 insertions(+), 16 deletions(-) create mode 100644 test/pdf-host.test.ts diff --git a/docs/fig-extract-integration.md b/docs/fig-extract-integration.md index 325868b..2b43224 100644 --- a/docs/fig-extract-integration.md +++ b/docs/fig-extract-integration.md @@ -115,9 +115,30 @@ const seeds = toFigureEntries(res, (p) => pageHeights[p]); 진행 중 렌더의 `RenderTask.cancel()`에서만 멈춘다. 문서를 바꾼 뒤에도 나가는 스캔이 **약 1페이지 분량**(페이지 캔버스 1장 + 그 페이지까지의 크롭)을 더 들고 있을 수 있다. "스캔 두 개가 끝까지" 대비 이득이 목적이고, 0이 목표가 아니다. -- **이전 `PDFDocumentProxy`를 `destroy()`하지 않는다** (#35). `PdfHost.#setDocument`가 `#doc`을 - 덮어쓸 뿐이라 한 세션에서 문서를 N번 열면 N개가 상주한다. **#34보다 큰 압력원**이다(문서를 열수록 - 단조 증가). v2.14.0에서는 크롭이 살아 있는 캔버스로 유지되므로 위 B7 실패에 직접 기여한다. + - ⚠ **엔진 쪽 미해결 (벤더링 시 upstream 확인 필요)**: 1차 패스의 `await page.getTextContent()` + 안에서 문서가 destroy되면(#35) 그 promise가 **영원히 settle되지 않는다** — pdf.js worker의 + `GetTextContent` 핸들러가 `task.terminated`면 sink를 error 처리하지 않고 빠지고, + `PDFPageProxy._destroy()`는 operator-list 스트림만 취소해 텍스트 스트림은 추적하지 않는다. + 결과적으로 그 스캔의 async frame이 pinned돼 페이지 텍스트 메타데이터가 영구 상주한다(크롭 + 캔버스는 아니다 — 2차 패스의 `getOperatorList`는 정상 거절한다). 문서 교체마다 반복되므로 + **단조 증가**다. 근본 해결은 엔진이 `getTextContent`를 abort signal과 race시키는 것이고, + Margin 쪽에서는 막을 수 없다. +- ~~이전 `PDFDocumentProxy`를 `destroy()`하지 않는다~~ → **해소 (#35)**. `PdfHost.#setDocument`가 + **뷰어·linkService를 새 문서로 전환한 뒤** 이전 문서를 `destroy()`한다(순서 반대면 뷰어가 방금 + 파괴된 문서를 렌더하려 한다). 정리 실패는 `console.warn`으로 삼켜 새 문서 로드를 막지 않는다. + - **로드 실패 시에는 이전 문서가 잠시 남는다**: `#setDocument`에 도달하지 못하므로 정리가 다음 + 성공 로드로 밀린다. `#doc`은 성공에서만 전진하므로 **누적되지 않고 최대 1개**다. + 실패 시점에 정리하지 않는 이유는 "화면에 떠 있어서"가 아니다(`setLoading`이 이미 뷰어를 + 감췄다) — **`viewer.setDocument(null)`을 부르지 않았으므로 `PDFViewer`·`PDFLinkService`가 + 여전히 그 문서를 가리키고 page view도 마운트된 채 재렌더 가능**하기 때문이다. 창 크기 변경 → + `refreshFitWidthIfNeeded` → `viewer.update()` → `PDFPageView.draw()` 경로가 파괴된 페이지를 + 렌더하려 하며 터진다. + - **실패한 로드 자신의 `PDFDocumentLoadingTask`·전용 워커는 별도로 정리한다**: pdf.js는 실패 시 + promise만 reject하고 task를 회수하지 않아, 파일 없음·권한 거부가 반복되면 **워커 스레드가 + 단조 증가**한다. `#awaitLoad`가 실패 경로에서 `loadingTask.destroy()`를 부른다. + - ⚠ **`GlobalWorkerOptions.workerPort`를 설정하면 이 정리가 위험해진다**: pdf.js가 + `PDFWorker.fromPort`로 모든 문서가 공유하는 싱글턴 워커를 넘기므로, 한 문서를 destroy하면 + 나머지 문서의 워커까지 종료된다. 문서당 워커 1개가 전제다. ## 주의사항 diff --git a/src/viewer/main.ts b/src/viewer/main.ts index 3f29754..dc1d026 100644 --- a/src/viewer/main.ts +++ b/src/viewer/main.ts @@ -147,7 +147,14 @@ let downloadName = 'document.pdf'; async function downloadCurrentPdf(): Promise { const doc = host.pdfDocument; if (!doc || downloadButton.disabled) return; - const data = await doc.getData(); + /* 내려받는 도중 사용자가 다른 PDF를 열면 이 문서는 destroy된다 (#35) — getData가 거절하므로 + * unhandled rejection이 되지 않게 삼킨다. 이미 사라진 문서라 재시도할 대상도 없다. */ + let data: Awaited>; + try { + data = await doc.getData(); + } catch { + return; + } // getData()의 Uint8Array는 BlobPart와 타입이 안 맞아 ArrayBuffer 사본으로 감싼다. const blob = new Blob([new Uint8Array(data)], { type: 'application/pdf' }); const url = URL.createObjectURL(blob); @@ -886,9 +893,10 @@ tocList.addEventListener('click', (event) => { if (!row) return; const item = outlineItems.find((candidate) => candidate.id === row.dataset.id); if (!item) return; + /* 이동 중 문서가 교체되면 getDestination이 거절한다 (#35) — 조용히 흘린다 */ void host.jumpToOutline(item).then(() => { if (!pinned) closePanel(); - }); + }).catch(() => {}); }); const file = readFileParam(); diff --git a/src/viewer/pdf-host.ts b/src/viewer/pdf-host.ts index 8b47da4..ee4c3c0 100644 --- a/src/viewer/pdf-host.ts +++ b/src/viewer/pdf-host.ts @@ -14,6 +14,9 @@ import type { PDFDocumentProxy } from 'pdfjs-dist/types/src/display/api'; import type { PageViewport } from 'pdfjs-dist/types/src/display/display_utils'; import type { DocId, DocMeta } from '../core/types'; +/* workerSrc만 설정한다 — `workerPort`는 절대 쓰지 말 것 (#35). workerPort를 주면 pdf.js가 + * `PDFWorker.fromPort`로 **모든 문서가 공유하는 싱글턴 워커**를 넘기고, 그러면 #releaseDocument의 + * destroy()가 다른 문서들의 워커까지 종료시킨다. 문서당 워커 1개라는 전제 위에 서 있는 코드다. */ GlobalWorkerOptions.workerSrc = workerSrc; export type OutlineNode = { @@ -100,7 +103,7 @@ export class PdfHost { docBaseUrl: url, isEvalSupported: false }); - const doc = await loadingTask.promise; + const doc = await this.#awaitLoad(loadingTask); this.#setDocument(doc, url); return doc; } @@ -111,11 +114,24 @@ export class PdfHost { data, isEvalSupported: false }); - const doc = await loadingTask.promise; + const doc = await this.#awaitLoad(loadingTask); this.#setDocument(doc); return doc; } + /* 로드가 실패하면 loadingTask를 명시로 정리한다 (#35). getDocument는 문서마다 전용 PDFWorker를 + * 띄우는데, 실패 시 pdf.js는 promise만 reject하고 task·worker를 회수하지 않는다 — 파일 없음·권한 + * 거부가 일상적인 UX라 실패 N번이 워커 스레드 N개로 쌓인다. 성공 경로는 #setDocument가 이전 + * 문서를 destroy할 때 함께 정리되므로 여기서 건드리지 않는다. */ + async #awaitLoad(loadingTask: { promise: Promise; destroy(): Promise }) { + try { + return await loadingTask.promise; + } catch (error) { + void loadingTask.destroy().catch(() => {}); + throw error; + } + } + async getTitle(fallback: string): Promise { if (!this.#doc) return fallback; try { @@ -201,22 +217,34 @@ export class PdfHost { this.viewer.update(); } + /* 문서 하나에 고정해서 읽는다 (#35). 원격 PDF의 outline은 아직 안 받은 range를 기다릴 수 있어 + * 이 함수가 수 초 매달릴 수 있는데, 그 사이 사용자가 다른 PDF를 열면 #releaseDocument가 이 문서를 + * destroy한다 — 그러면 getOutline이 AbortException으로 거절하고, 그 거절이 **이전 로드의 catch**로 + * 흘러가 방금 열린 새 문서 위에 오류 화면을 띄운다. 거절을 삼키고, 교체를 감지하면 빈 목록으로 + * 빠진다. this.#doc을 다시 읽지 않는 것도 같은 이유다(교체 후 새 문서에 옛 dest를 풀지 않는다). */ async getOutlineItems(): Promise { - if (!this.#doc) return []; - const outline = (await this.#doc.getOutline()) as OutlineNode[] | null; - if (!outline?.length) return []; + const doc = this.#doc; + if (!doc) return []; + let outline: OutlineNode[] | null; + try { + outline = (await doc.getOutline()) as OutlineNode[] | null; + } catch { + return []; // 파괴된 문서이거나 outline을 못 읽는 문서 — TOC 없음으로 취급 + } + if (this.#doc !== doc || !outline?.length) return []; const items: FlatOutlineItem[] = []; let nextId = 0; const walk = async (nodes: OutlineNode[], depth: number): Promise => { for (const node of nodes) { + if (this.#doc !== doc) return; const item: FlatOutlineItem = { id: `toc-${nextId++}`, title: node.title, depth, dest: node.dest, url: node.url, - page: await this.#resolveDestPage(node.dest) + page: await this.#resolveDestPage(node.dest, doc) }; items.push(item); if (node.items?.length) { @@ -226,7 +254,7 @@ export class PdfHost { }; await walk(outline, 0); - return items; + return this.#doc === doc ? items : []; } async jumpToOutline(item: FlatOutlineItem): Promise { @@ -242,6 +270,7 @@ export class PdfHost { } #setDocument(doc: PDFDocumentProxy, url?: string): void { + const previous = this.#doc; this.#doc = doc; const linkService = this.linkService as PDFLinkService & { setDocument(pdfDocument: PDFDocumentProxy, baseUrl?: string | null): void; @@ -249,17 +278,37 @@ export class PdfHost { linkService.setDocument(doc, url ?? null); this.viewer.setDocument(doc); this.refreshLayoutSoon(); + /* 이전 문서는 **뷰어·linkService가 새 문서로 전환된 뒤에** 정리한다 (#35). pdf.js는 + * PDFDocumentProxy를 destroy()하지 않으면 워커 측 자원과 페이지 프록시를 계속 붙들고 있어 + * GC 대상이 되지 않는다 — 한 세션에서 문서를 N번 열면 N개가 그대로 상주하고, 벤더링본 + * v2.14.0에서는 크롭도 살아 있는 캔버스라 Chrome이 캔버스 백킹 스토어를 회수하는 조건 + * (엔진 백로그 B7)에 직접 기여한다. + * 순서가 중요하다 — 전환 전에 destroy하면 뷰어가 방금 파괴된 문서를 렌더하려 한다. */ + if (previous && previous !== doc) void this.#releaseDocument(previous); + } + + /** 이전 문서의 워커 자원을 반환한다. 실패해도 새 문서 로드를 막지 않는다 (#35). */ + async #releaseDocument(doc: PDFDocumentProxy): Promise { + try { + await doc.destroy(); + } catch (error) { + console.warn('이전 PDF 문서를 정리하지 못했습니다', error); + } } - async #resolveDestPage(dest: string | unknown[] | null): Promise { - if (!this.#doc || !dest) return null; + /** doc을 명시로 받는다 — 호출 중 문서가 교체돼도 옛 dest를 새 문서에 풀지 않는다 (#35). */ + async #resolveDestPage( + dest: string | unknown[] | null, + doc: PDFDocumentProxy | null = this.#doc + ): Promise { + if (!doc || !dest) return null; try { - const destArray = typeof dest === 'string' ? await this.#doc.getDestination(dest) : dest; + const destArray = typeof dest === 'string' ? await doc.getDestination(dest) : dest; if (!destArray?.length) return null; const first = destArray[0]; if (typeof first === 'number') return first + 1; if (first && typeof first === 'object' && 'num' in first && 'gen' in first) { - return (await this.#doc.getPageIndex(first as { num: number; gen: number })) + 1; + return (await doc.getPageIndex(first as { num: number; gen: number })) + 1; } return null; } catch { diff --git a/test/pdf-host.test.ts b/test/pdf-host.test.ts new file mode 100644 index 0000000..1f3c083 --- /dev/null +++ b/test/pdf-host.test.ts @@ -0,0 +1,177 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { PDFDocumentProxy } from 'pdfjs-dist/types/src/display/api'; + +/* pdf.js는 워커를 띄우고 실제 DOM에 렌더하므로 통째로 대역을 세운다 — 여기서 검증하는 것은 + * PdfHost의 문서 교체 순서와 이전 문서 정리(#35)뿐이다. */ +const getDocument = vi.fn(); +const linkServiceSetDocument = vi.fn(); +const viewerSetDocument = vi.fn(); + +vi.mock('pdfjs-dist', () => ({ + GlobalWorkerOptions: { workerSrc: '' }, + getDocument: (...args: unknown[]) => getDocument(...args), + version: '4.10.38' +})); + +vi.mock('pdfjs-dist/build/pdf.worker.mjs?url', () => ({ default: 'worker.mjs' })); + +vi.mock('pdfjs-dist/web/pdf_viewer.mjs', () => ({ + EventBus: class { + on(): void {} + }, + LinkTarget: { BLANK: 2 }, + PDFLinkService: class { + setViewer(): void {} + setDocument(...args: unknown[]): void { + linkServiceSetDocument(...args); + } + }, + PDFViewer: class { + currentPageNumber = 1; + currentScaleValue = ''; + setDocument(...args: unknown[]): void { + viewerSetDocument(...args); + } + } +})); + +/* 정적 import는 위 vi.mock 팩토리가 닫아 잡는 const들(:8-10) 위로 호이스팅돼 + * "Cannot access 'getDocument' before initialization"으로 죽는다 — 동적 import여야 한다. */ +const { PdfHost } = await import('../src/viewer/pdf-host'); + +/** destroy 호출을 기록하는 문서 대역. numPages만 실제로 읽힌다. */ +function makeDoc(label: string, destroy: () => Promise = async () => {}): PDFDocumentProxy { + return { label, numPages: 3, destroy: vi.fn(destroy) } as unknown as PDFDocumentProxy; +} +const destroyOf = (doc: PDFDocumentProxy) => (doc as unknown as { destroy: ReturnType }).destroy; + +function makeHost(): InstanceType { + return new PdfHost({ + container: {} as HTMLDivElement, + viewer: {} as HTMLDivElement + }); +} + +const asFile = () => ({ arrayBuffer: async () => new ArrayBuffer(0) }) as unknown as File; + +/** loadFile 경로로 문서를 주입한다 (File.arrayBuffer만 필요). */ +async function load(host: InstanceType, doc: PDFDocumentProxy): Promise { + getDocument.mockReturnValueOnce({ promise: Promise.resolve(doc), destroy: vi.fn(async () => {}) }); + await host.loadFile(asFile()); +} + +describe('PdfHost document lifecycle', () => { + const originalWindow = globalThis.window; + + beforeEach(() => { + Object.defineProperty(globalThis, 'window', { + configurable: true, + value: { requestAnimationFrame: () => 0 } + }); + }); + + afterEach(() => { + Object.defineProperty(globalThis, 'window', { configurable: true, value: originalWindow }); + /* clearAllMocks는 호출 기록만 지운다 — 테스트 안에서 심은 mockImplementation과 console spy가 + * 다음 테스트로 새 나가므로 restore여야 한다. */ + vi.restoreAllMocks(); + getDocument.mockReset(); + linkServiceSetDocument.mockReset(); + viewerSetDocument.mockReset(); + }); + + it('keeps the first document alive', async () => { + const host = makeHost(); + const first = makeDoc('first'); + + await load(host, first); + + expect(host.pdfDocument).toBe(first); + expect(destroyOf(first)).not.toHaveBeenCalled(); + }); + + it('destroys the previous document and never the current one (#35)', async () => { + const host = makeHost(); + const first = makeDoc('first'); + const second = makeDoc('second'); + + await load(host, first); + await load(host, second); + + expect(destroyOf(first)).toHaveBeenCalledTimes(1); + expect(destroyOf(second)).not.toHaveBeenCalled(); + expect(host.pdfDocument).toBe(second); + }); + + it('destroys the previous document only after the viewer switched over (#35)', async () => { + const order: string[] = []; + const host = makeHost(); + const first = makeDoc('first', async () => { order.push('destroy:first'); }); + const second = makeDoc('second'); + viewerSetDocument.mockImplementation((doc: PDFDocumentProxy) => { + order.push(`viewer:${(doc as unknown as { label: string }).label}`); + }); + linkServiceSetDocument.mockImplementation((doc: PDFDocumentProxy) => { + order.push(`link:${(doc as unknown as { label: string }).label}`); + }); + + await load(host, first); + await load(host, second); + await vi.waitFor(() => expect(order).toContain('destroy:first')); + + /* pdf.js의 PDFViewer.setDocument는 나가는 문서를 **동기로** 정리한다(render/textLayer cancel → + * _resetView). 그 정리가 옛 proxy를 만지므로 destroy가 먼저 오면 파괴된 문서를 건드린다. + * 대역은 그 내부 동작을 재현하지 않으므로 여기서 고정하는 것은 **호출 순서**다. */ + expect(order.indexOf('destroy:first')).toBeGreaterThan(order.indexOf('viewer:second')); + expect(order).toEqual([ + 'link:first', 'viewer:first', + 'link:second', 'viewer:second', + 'destroy:first' + ]); + }); + + it('survives a document that fails to destroy (#35)', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const host = makeHost(); + const first = makeDoc('first', async () => { throw new Error('worker already gone'); }); + const second = makeDoc('second'); + + await load(host, first); + await load(host, second); + + /* 정리 실패는 warn 한 줄로 끝나고 새 문서 전환은 완료돼 있어야 한다. destroy는 void 처리라 + * load 자체는 어차피 resolve하므로 그것만으로는 아무것도 증명하지 못한다 — 여기가 본체다. */ + await vi.waitFor(() => expect(warn).toHaveBeenCalledTimes(1)); + expect(host.pdfDocument).toBe(second); + expect(viewerSetDocument).toHaveBeenLastCalledWith(second); + }); + + it('destroys the loading task when a load fails, so no worker is orphaned (#35)', async () => { + const host = makeHost(); + const first = makeDoc('first'); + await load(host, first); + + const taskDestroy = vi.fn(async () => {}); + getDocument.mockReturnValueOnce({ + promise: Promise.reject(new Error('missing file')), + destroy: taskDestroy + }); + + await expect(host.loadFile(asFile())).rejects.toThrow('missing file'); + await vi.waitFor(() => expect(taskDestroy).toHaveBeenCalledTimes(1)); + /* 실패한 로드는 화면에 남아 있는 문서를 건드리지 않는다 — 정리는 다음 성공 로드로 밀린다 */ + expect(host.pdfDocument).toBe(first); + expect(destroyOf(first)).not.toHaveBeenCalled(); + }); + + it('does not destroy a document that is being re-set as the same instance (#35)', async () => { + const host = makeHost(); + const doc = makeDoc('same'); + + await load(host, doc); + await load(host, doc); + + expect(destroyOf(doc)).not.toHaveBeenCalled(); + expect(host.pdfDocument).toBe(doc); + }); +}); From 1599a0b40cff9547532f54c0f19cc643f844fdc6 Mon Sep 17 00:00:00 2001 From: onetwothr1 Date: Tue, 28 Jul 2026 17:59:19 +0900 Subject: [PATCH 2/4] =?UTF-8?q?test:=20getOutlineItems=EC=9D=98=20?= =?UTF-8?q?=EB=AC=B8=EC=84=9C=20=EA=B5=90=EC=B2=B4=20=EA=B0=80=EB=93=9C?= =?UTF-8?q?=EB=A5=BC=20=EB=8B=A8=EC=9C=84=20=ED=85=8C=EC=8A=A4=ED=8A=B8?= =?UTF-8?q?=EB=A1=9C=20=EB=8D=AE=EB=8A=94=EB=8B=A4?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 이 가드는 #35에서 가장 위험한 지점(destroy가 유발한 거절이 이전 로드의 catch로 흘러가 새 문서 위에 오류 화면을 띄움)을 막는데, 수동 QA로만 확인 가능했다. 로컬 파일은 getOutline이 밀리초에 끝나 경합 창이 사실상 0이라 재현되지 않고, 원격 URL + 네트워크 스로틀링이 필요했다. - 조회 실패가 예외로 새어나가지 않고 빈 목차가 된다 - 조회 중 문서가 교체되면 옛 목차로 새 화면을 덮지 않는다 - happy path(중첩 목차 평탄화 + dest→page 해석) — #resolveDestPage가 doc을 명시로 받도록 바꾼 것의 회귀 가드 dev 기준으로 되돌리면 9개 중 6개가 실패함을 확인했다 (나머지 3개는 수정 전에도 통과가 맞는 회귀 가드다). typecheck 0 · vitest 38/38 · build 0 Co-Authored-By: Claude Opus 5 --- test/pdf-host.test.ts | 51 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/test/pdf-host.test.ts b/test/pdf-host.test.ts index 1f3c083..67f50c6 100644 --- a/test/pdf-host.test.ts +++ b/test/pdf-host.test.ts @@ -164,6 +164,57 @@ describe('PdfHost document lifecycle', () => { expect(destroyOf(first)).not.toHaveBeenCalled(); }); + it('flattens the outline of the current document', async () => { + const host = makeHost(); + const doc = makeDoc('only'); + Object.assign(doc, { + getOutline: async () => [ + { title: 'Ch 1', dest: 'ch1', url: null, items: [{ title: 'Ch 1.1', dest: null, url: null }] } + ], + getDestination: async () => [{ num: 5, gen: 0 }], + getPageIndex: async () => 2 + }); + await load(host, doc); + + const items = await host.getOutlineItems(); + expect(items.map((item) => [item.title, item.depth, item.page])).toEqual([ + ['Ch 1', 0, 3], + ['Ch 1.1', 1, null] + ]); + }); + + it('returns no outline instead of throwing when the read fails (#35)', async () => { + const host = makeHost(); + const doc = makeDoc('only'); + /* destroy된 문서의 getOutline은 AbortException으로 거절한다. 이게 새어나가면 **이전 로드의 + * catch**로 흘러가 방금 열린 새 문서 위에 오류 화면을 띄운다 — 그래서 삼켜야 한다. */ + Object.assign(doc, { getOutline: async () => { throw new Error('Worker was terminated.'); } }); + await load(host, doc); + + await expect(host.getOutlineItems()).resolves.toEqual([]); + }); + + it('discards an outline whose document was swapped out mid-read (#35)', async () => { + const host = makeHost(); + let releaseOutline!: (nodes: unknown[]) => void; + const first = makeDoc('first'); + Object.assign(first, { + getOutline: () => new Promise((resolve) => { releaseOutline = resolve as typeof releaseOutline; }), + getDestination: async () => [{ num: 1, gen: 0 }], + getPageIndex: async () => 0 + }); + const second = makeDoc('second'); + + await load(host, first); + const pending = host.getOutlineItems(); + await load(host, second); // 조회 중 문서 교체 + releaseOutline([{ title: 'stale', dest: 'x', url: null }]); + + /* 옛 문서의 목차로 새 문서의 TOC를 덮지 않는다 */ + await expect(pending).resolves.toEqual([]); + expect(host.pdfDocument).toBe(second); + }); + it('does not destroy a document that is being re-set as the same instance (#35)', async () => { const host = makeHost(); const doc = makeDoc('same'); From e2884d432d972a09b9f7e40e2993713fe10c2ba7 Mon Sep 17 00:00:00 2001 From: onetwothr1 Date: Tue, 28 Jul 2026 22:16:34 +0900 Subject: [PATCH 3/4] =?UTF-8?q?fix:=20=EB=8A=A6=EA=B2=8C=20=EB=81=9D?= =?UTF-8?q?=EB=82=9C=20=EB=A1=9C=EB=93=9C=EA=B0=80=20=EC=82=AC=EC=9A=A9?= =?UTF-8?q?=EC=9E=90=EA=B0=80=20=EB=A7=88=EC=A7=80=EB=A7=89=EC=9C=BC?= =?UTF-8?q?=EB=A1=9C=20=EC=97=B0=20=EB=AC=B8=EC=84=9C=EB=A5=BC=20=ED=8C=8C?= =?UTF-8?q?=EA=B4=B4=ED=95=98=EC=A7=80=20=EC=95=8A=EA=B2=8C=20=ED=95=9C?= =?UTF-8?q?=EB=8B=A4?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit 지적. 느린 URL 로드(A) 중에 파일(B)을 드롭하면 B가 먼저 적용되는데, 그 뒤 A가 완료되면 #setDocument(A)가 화면에 떠 있는 B를 destroy한다. 이 겹침 자체는 선재 결함이지만(#37), 이전에는 "옛 문서가 화면을 덮어씀"이라는 표시 문제였던 것이 이 PR의 destroy 때문에 "사용자가 연 문서가 파괴됨"으로 승격된다. FiguresTab이 B를 스캔 중이면 abort되지 않은 채 파괴된 문서를 만나 오류 UI까지 뜬다. 그래서 destroy를 추가한 이 PR에서 함께 막는다. - PdfHost가 로드마다 일련번호를 부여하고, 완료 시점에 최신이 아니면 방금 받은 문서를 스스로 destroy한 뒤 PdfLoadSupersededError로 물러난다 - main.ts의 두 catch가 이 신호를 오류로 취급하지 않고 조용히 반환한다 — 화면은 더 최신 로드의 것이므로 건드리면 안 된다 표시 상태·TOC·docData의 stale 갱신은 main.ts 레벨 문제라 #37에 남겨 둔다. typecheck 0 · vitest 39/39 · build 0 Co-Authored-By: Claude Opus 5 --- src/viewer/main.ts | 6 +++++- src/viewer/pdf-host.ts | 38 +++++++++++++++++++++++++++++++++++--- test/pdf-host.test.ts | 26 ++++++++++++++++++++++++++ 3 files changed, 66 insertions(+), 4 deletions(-) diff --git a/src/viewer/main.ts b/src/viewer/main.ts index dc1d026..8211ea3 100644 --- a/src/viewer/main.ts +++ b/src/viewer/main.ts @@ -17,7 +17,7 @@ import type { Highlight, Memo, PenColor } from '../core/types'; import { HighlightOverlay } from './overlay-highlights'; import { FiguresTab } from './panel/tab-figures'; import { MemoTab } from './panel/tab-memos'; -import { type FlatOutlineItem, PdfHost } from './pdf-host'; +import { type FlatOutlineItem, PdfHost, isLoadSuperseded } from './pdf-host'; const PANEL_WIDTH_KEY = 'margin:panelWidth'; const PANEL_MIN_WIDTH = 264; @@ -281,6 +281,8 @@ async function loadUrl(file: string): Promise { setPageUi(host.currentPage, host.pageCount); renderToc(await host.getOutlineItems()); } catch (error) { + /* 이 로드가 더 최신 로드로 밀려났다면 화면은 그쪽 것이다 — 건드리지 않고 물러난다 (#35) */ + if (isLoadSuperseded(error)) return; figuresTab.setDocument(null); if (isLocalFile && isMissingPdfError(error)) { showMissingFileState(file); @@ -306,6 +308,8 @@ async function loadSelectedFile(file: File): Promise { setPageUi(host.currentPage, host.pageCount); renderToc(await host.getOutlineItems()); } catch (error) { + /* 이 로드가 더 최신 로드로 밀려났다면 화면은 그쪽 것이다 — 건드리지 않고 물러난다 (#35) */ + if (isLoadSuperseded(error)) return; figuresTab.setDocument(null); setError(error); } diff --git a/src/viewer/pdf-host.ts b/src/viewer/pdf-host.ts index ee4c3c0..ca43754 100644 --- a/src/viewer/pdf-host.ts +++ b/src/viewer/pdf-host.ts @@ -40,6 +40,22 @@ type PdfHostElements = { viewer: HTMLDivElement; }; +/** + * 로드가 끝나기 전에 사용자가 다른 문서를 요청해 이 로드가 밀려났다는 신호 (#35). + * **오류가 아니다** — 호출 측은 화면을 건드리지 말고 조용히 물러나야 한다. 이걸 일반 실패로 + * 처리하면 정상 로드된 새 문서 위에 오류 화면이 뜬다. + */ +export class PdfLoadSupersededError extends Error { + constructor() { + super('이 PDF 로드는 더 최신 로드로 대체되었습니다.'); + this.name = 'PdfLoadSupersededError'; + } +} + +export const isLoadSuperseded = (error: unknown): boolean => + typeof error === 'object' && error !== null + && (error as { name?: unknown }).name === 'PdfLoadSupersededError'; + type PdfHostCallbacks = { onPageChange?: (page: number, pageCount: number) => void; onScaleChange?: (scale: number, presetValue?: string) => void; @@ -52,6 +68,8 @@ export class PdfHost { readonly version = pdfjsVersion; #doc: PDFDocumentProxy | null = null; + /** 로드 요청 일련번호 — 완료 시점에 자기가 아직 최신인지 판별한다 (#35) */ + #loadSeq = 0; #callbacks: PdfHostCallbacks; constructor(elements: PdfHostElements, callbacks: PdfHostCallbacks = {}) { @@ -98,24 +116,38 @@ export class PdfHost { } async loadUrl(url: string): Promise { + const seq = ++this.#loadSeq; const loadingTask = getDocument({ url, docBaseUrl: url, isEvalSupported: false }); const doc = await this.#awaitLoad(loadingTask); - this.#setDocument(doc, url); - return doc; + return this.#adopt(doc, seq, url); } async loadFile(file: File): Promise { + const seq = ++this.#loadSeq; const data = new Uint8Array(await file.arrayBuffer()); const loadingTask = getDocument({ data, isEvalSupported: false }); const doc = await this.#awaitLoad(loadingTask); - this.#setDocument(doc); + return this.#adopt(doc, seq); + } + + /* 늦게 끝난 로드가 사용자가 **마지막으로 요청한** 문서를 밀어내지 않게 한다 (#35). + * 느린 URL(A)을 로드하는 중에 파일(B)을 드롭하면 B가 먼저 적용되는데, 그 뒤 A가 완료되면 + * #setDocument(A)가 화면에 떠 있는 B를 destroy해 버린다 — 이 destroy는 이 PR이 추가한 것이라 + * 여기서 함께 막는다. 밀려난 쪽은 방금 받은 자기 문서를 스스로 버리고 물러난다. + * (표시 상태·TOC·docData의 stale 갱신은 main.ts 레벨 문제라 #37에 남아 있다.) */ + #adopt(doc: PDFDocumentProxy, seq: number, url?: string): PDFDocumentProxy { + if (seq !== this.#loadSeq) { + void this.#releaseDocument(doc); + throw new PdfLoadSupersededError(); + } + this.#setDocument(doc, url); return doc; } diff --git a/test/pdf-host.test.ts b/test/pdf-host.test.ts index 67f50c6..c77b298 100644 --- a/test/pdf-host.test.ts +++ b/test/pdf-host.test.ts @@ -215,6 +215,32 @@ describe('PdfHost document lifecycle', () => { expect(host.pdfDocument).toBe(second); }); + it('discards a late load instead of destroying the document the user actually opened (#35)', async () => { + const host = makeHost(); + let releaseSlow!: (doc: PDFDocumentProxy) => void; + const slow = makeDoc('slow'); + const fast = makeDoc('fast'); + + /* 느린 URL 로드(A)가 진행 중일 때 사용자가 파일(B)을 드롭하는 상황 */ + getDocument.mockReturnValueOnce({ + promise: new Promise((resolve) => { releaseSlow = resolve; }), + destroy: vi.fn(async () => {}) + }); + const slowLoad = host.loadUrl('https://example.com/slow.pdf'); + await load(host, fast); + expect(host.pdfDocument).toBe(fast); + + releaseSlow(slow); + + /* 밀려난 로드는 오류가 아니라 "물러남" 신호로 끝나고, 자기가 받은 문서를 스스로 버린다 */ + await expect(slowLoad).rejects.toMatchObject({ name: 'PdfLoadSupersededError' }); + await vi.waitFor(() => expect(destroyOf(slow)).toHaveBeenCalledTimes(1)); + /* 사용자가 마지막으로 요청한 문서는 살아 있고 화면에도 그대로다 */ + expect(destroyOf(fast)).not.toHaveBeenCalled(); + expect(host.pdfDocument).toBe(fast); + expect(viewerSetDocument).toHaveBeenLastCalledWith(fast); + }); + it('does not destroy a document that is being re-set as the same instance (#35)', async () => { const host = makeHost(); const doc = makeDoc('same'); From 8e96f41bea79f9af93ca330418e19b1a71820be8 Mon Sep 17 00:00:00 2001 From: onetwothr1 Date: Tue, 28 Jul 2026 22:31:46 +0900 Subject: [PATCH 4/4] =?UTF-8?q?fix:=20=EB=B0=80=EB=A0=A4=EB=82=9C=20?= =?UTF-8?q?=EB=A1=9C=EB=93=9C=EC=9D=98=20=EC=8B=A4=ED=8C=A8=EB=8F=84=20sup?= =?UTF-8?q?erseded=EB=A1=9C=20=EC=A0=84=ED=99=98=ED=95=9C=EB=8B=A4?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit 후속 지적. #adopt는 성공 경로만 막았고 실패 경로는 그대로 원본 오류를 올려보냈다. B가 먼저 성공한 뒤 A(seq 1)가 실패하면 main.ts의 catch가 일반 오류로 처리해 figuresTab.setDocument(null) + setError()를 실행하고, 그 화면에는 사용자가 실제로 연 B가 떠 있다. #awaitLoad가 seq를 받아, 실패 시에도 최신이 아니면 PdfLoadSupersededError로 바꿔 던진다. loadingTask 정리는 두 경우 모두 그대로 수행한다. 밀려나지 않은 로드의 실패는 원본 오류를 유지해야 오류 화면이 정상 동작하므로 그 회귀 가드도 함께 넣었다. typecheck 0 · vitest 41/41 · build 0 Co-Authored-By: Claude Opus 5 --- src/viewer/pdf-host.ts | 14 ++++++++++---- test/pdf-host.test.ts | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/src/viewer/pdf-host.ts b/src/viewer/pdf-host.ts index ca43754..41c5728 100644 --- a/src/viewer/pdf-host.ts +++ b/src/viewer/pdf-host.ts @@ -122,7 +122,7 @@ export class PdfHost { docBaseUrl: url, isEvalSupported: false }); - const doc = await this.#awaitLoad(loadingTask); + const doc = await this.#awaitLoad(loadingTask, seq); return this.#adopt(doc, seq, url); } @@ -133,7 +133,7 @@ export class PdfHost { data, isEvalSupported: false }); - const doc = await this.#awaitLoad(loadingTask); + const doc = await this.#awaitLoad(loadingTask, seq); return this.#adopt(doc, seq); } @@ -155,12 +155,18 @@ export class PdfHost { * 띄우는데, 실패 시 pdf.js는 promise만 reject하고 task·worker를 회수하지 않는다 — 파일 없음·권한 * 거부가 일상적인 UX라 실패 N번이 워커 스레드 N개로 쌓인다. 성공 경로는 #setDocument가 이전 * 문서를 destroy할 때 함께 정리되므로 여기서 건드리지 않는다. */ - async #awaitLoad(loadingTask: { promise: Promise; destroy(): Promise }) { + async #awaitLoad( + loadingTask: { promise: Promise; destroy(): Promise }, + seq: number + ) { try { return await loadingTask.promise; } catch (error) { void loadingTask.destroy().catch(() => {}); - throw error; + /* 이미 밀려난 로드의 **실패**도 화면에 띄우면 안 된다 (#35). 그대로 올려보내면 호출 측이 + * 일반 오류로 처리해 figuresTab을 비우고 오류 화면을 띄우는데, 그 화면에는 사용자가 + * 실제로 연 다른 문서가 떠 있다. 성공 경로의 #adopt와 같은 판정을 실패 경로에도 적용한다. */ + throw seq !== this.#loadSeq ? new PdfLoadSupersededError() : error; } } diff --git a/test/pdf-host.test.ts b/test/pdf-host.test.ts index c77b298..ff66129 100644 --- a/test/pdf-host.test.ts +++ b/test/pdf-host.test.ts @@ -241,6 +241,40 @@ describe('PdfHost document lifecycle', () => { expect(viewerSetDocument).toHaveBeenLastCalledWith(fast); }); + it('reports a late load FAILURE as superseded, not as an error (#35)', async () => { + const host = makeHost(); + let failSlow!: (reason: Error) => void; + const taskDestroy = vi.fn(async () => {}); + const fast = makeDoc('fast'); + + getDocument.mockReturnValueOnce({ + promise: new Promise((_resolve, reject) => { failSlow = reject; }), + destroy: taskDestroy + }); + const slowLoad = host.loadUrl('https://example.com/slow.pdf'); + await load(host, fast); + + failSlow(Object.assign(new Error('missing'), { name: 'MissingPDFException' })); + + /* 원본 오류를 그대로 올려보내면 호출 측이 figuresTab을 비우고 오류 화면을 띄운다 — + * 그 화면에는 사용자가 실제로 연 문서가 떠 있다. superseded로 바꿔야 조용히 물러난다. */ + await expect(slowLoad).rejects.toMatchObject({ name: 'PdfLoadSupersededError' }); + await vi.waitFor(() => expect(taskDestroy).toHaveBeenCalledTimes(1)); + expect(host.pdfDocument).toBe(fast); + expect(destroyOf(fast)).not.toHaveBeenCalled(); + }); + + it('still reports a current load failure as a real error (#35)', async () => { + const host = makeHost(); + getDocument.mockReturnValueOnce({ + promise: Promise.reject(Object.assign(new Error('missing'), { name: 'MissingPDFException' })), + destroy: vi.fn(async () => {}) + }); + + /* 밀려나지 않은 로드의 실패는 그대로 올라가야 한다 — 안 그러면 오류 화면이 영영 안 뜬다 */ + await expect(host.loadFile(asFile())).rejects.toMatchObject({ name: 'MissingPDFException' }); + }); + it('does not destroy a document that is being re-set as the same instance (#35)', async () => { const host = makeHost(); const doc = makeDoc('same');