From 2e88ccdea7dde5a0f8b2c59e8fd9180e614d5e52 Mon Sep 17 00:00:00 2001 From: Huang Xin Date: Wed, 14 Oct 2026 20:35:15 +0800 Subject: [PATCH] fix(reader): release closed reader views from the cached book document, closes #6710 (#6788) * fix(reader): release closed reader views from the cached book document, closes #6710 The library keeps each opened book's document in bookDataStore for the next open. FoliateViewer and foliate-js's paginator both added listeners to that document's transformTarget on every open and never removed them, so each closed reader (its foliate-view, iframes, section documents and their listeners) stayed alive for the rest of the session, one more per open. A restart was the only thing that released them, as the report describes. FoliateViewer now removes its load/data listeners on unmount, and the paginator removes its listener in destroy() (foliate-js 49b4b05). Verified: - foliate-viewer-transform-listeners.test.tsx fails before (2 listeners left after unmount) and passes after; paginator-transform-listener .browser.test.ts fails before (1 left after destroy) and passes after. - Production web build, Playwright Chromium, 40 EPUBs, one book opened and closed 12 times, measured after forced GC: before, each cycle left +1 frame, +1 document, ~+79 DOM nodes, ~+16 listeners and ~+0.75 MB heap; after, all flat (heap ~+0.3 MB/cycle). A heap snapshot shows no closed foliate-view reachable from app code after the fix. - pnpm lint, format:check, the related vitest files and the 37 paginator browser test files pass. Not run: iOS (the reporter's platform), no device or simulator here. The library return time did not degrade in desktop Chromium before the fix, so the link between this leak and the reported delay is not shown on iOS. Co-Authored-By: Claude Opus 5.5 * chore(deps): move foliate-js to the merged readest/foliate-js#128 Same tree as the 49b4b05 branch head this PR was tested with; the squash merge only changed the commit id. Co-Authored-By: Claude Opus 5.5 * fix(reader): drop transformTarget listeners added after the reader closed openBook registers its transformTarget listeners after awaiting view.open(). If the reader unmounts while that is pending, its cleanup has already run, so the listeners were added afterwards and kept the closed view alive, the same leak as #6710. They are now removed at once when the reader has already closed. Found by CodeRabbit on #6788. Verified: the new "closes while the book is opening" test fails without the change (expected 2 to be +0) and passes with it; the 90 reader test files pass; pnpm lint passes. Not run: iOS. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- ...oliate-viewer-transform-listeners.test.tsx | 147 ++++++++++++++++++ ...ginator-transform-listener.browser.test.ts | 35 +++++ .../app/reader/components/FoliateViewer.tsx | 23 ++- packages/foliate-js | 2 +- 4 files changed, 203 insertions(+), 4 deletions(-) create mode 100644 apps/readest-app/src/__tests__/app/reader/foliate-viewer-transform-listeners.test.tsx create mode 100644 apps/readest-app/src/__tests__/document/paginator-transform-listener.browser.test.ts diff --git a/apps/readest-app/src/__tests__/app/reader/foliate-viewer-transform-listeners.test.tsx b/apps/readest-app/src/__tests__/app/reader/foliate-viewer-transform-listeners.test.tsx new file mode 100644 index 000000000..eb1c91dc8 --- /dev/null +++ b/apps/readest-app/src/__tests__/app/reader/foliate-viewer-transform-listeners.test.tsx @@ -0,0 +1,147 @@ +import { render, waitFor } from '@testing-library/react'; +import { expect, it, vi } from 'vitest'; +import type { ComponentProps } from 'react'; +import FoliateViewer from '@/app/reader/components/FoliateViewer'; + +const { readerState, openGate } = vi.hoisted(() => { + return { + openGate: { pending: null as Promise | null, started: false }, + readerState: { + setProgress: vi.fn(), + getViewState: () => ({}), + getViewSettings: () => ({ writingMode: 'auto' }), + setView: () => {}, + setViewInited: () => {}, + }, + }; +}); + +vi.mock('@/store/readerStore', () => ({ + useReaderStore: (select: (s: typeof readerState) => unknown) => select(readerState), +})); +vi.mock('@/store/bookDataStore', () => ({ + useBookDataStore: (select: (s: { getBookData: () => null }) => unknown) => + select({ getBookData: () => null }), +})); +vi.mock('@/store/parallelViewStore', () => ({ useParallelViewStore: () => () => [] })); +vi.mock('@/store/settingsStore', () => ({ + useSettingsStore: Object.assign(() => ({ settings: {} }), { getState: () => ({ settings: {} }) }), +})); +vi.mock('@/store/themeStore', () => ({ useThemeStore: () => ({}) })); +vi.mock('@/store/customFontStore', () => ({ + useCustomFontStore: () => ({ getLoadedFonts: () => [], getAvailableFonts: () => [] }), +})); +vi.mock('@/context/EnvContext', () => ({ useEnv: () => ({}) })); +vi.mock('next/navigation', () => ({ useSearchParams: () => null })); +vi.mock('@/hooks/useTranslation', () => ({ useTranslation: () => (text: string) => text })); +vi.mock('@/libs/document', () => ({})); +vi.mock('foliate-js/view.js', () => ({})); +vi.mock('@/types/view', () => ({ + wrappedFoliateView: (view: HTMLElement & { book?: unknown }) => { + const anyCall: object = new Proxy(() => anyCall, { get: () => anyCall, apply: () => anyCall }); + return Object.assign(view, { + open: async (book: unknown) => { + view.book = book; + openGate.started = true; + await openGate.pending; + }, + init: async () => {}, + goToFraction: async () => {}, + renderer: anyCall, + }); + }, +})); +vi.mock('@/utils/style', async (importOriginal) => ({ + ...(await importOriginal()), + getStyles: () => '', +})); +vi.mock('@/services/transformService', () => ({})); +vi.mock('@/app/reader/utils/wordlensSection', () => ({})); +vi.mock('@/app/reader/hooks/useFoliateEvents', () => ({ useFoliateEvents: vi.fn() })); +vi.mock('@/app/reader/hooks/useBrightnessGesture', () => ({ useBrightnessGesture: () => ({}) })); +vi.mock('@/app/reader/hooks/useAutoScroll', () => ({ useAutoScroll: () => ({}) })); +vi.mock('@/app/reader/hooks/useAutoScrollSpeedGesture', () => ({ + useAutoScrollSpeedGesture: () => ({}), +})); +vi.mock('@/app/reader/hooks/useMiddleClickAutoscroll', () => ({ + useMiddleClickAutoscroll: () => null, +})); +vi.mock('@/app/reader/hooks/useKOSync', () => ({ useKOSync: () => ({}) })); +vi.mock('@/app/reader/hooks/useIframeEvents', () => ({ + useMouseEvent: () => ({}), + useTouchEvent: () => ({}), + useOpenMediaEvent: () => {}, +})); +vi.mock('@/app/reader/hooks/useCapturedTurn', () => ({ + useCapturedTurn: () => {}, + applyPageTurnAttributes: () => {}, +})); +vi.mock('@/app/reader/hooks/usePagination', () => ({ usePagination: () => ({}) })); +vi.mock('@/app/reader/hooks/useProgressSync', () => ({ useProgressSync: () => {} })); +vi.mock('@/app/reader/hooks/useABSProgressSync', () => ({ useABSProgressSync: () => {} })); +vi.mock('@/app/reader/hooks/useProgressAutoSave', () => ({ useProgressAutoSave: () => {} })); +vi.mock('@/app/reader/hooks/useAutoSaveBookCover', () => ({ useBookCoverAutoSave: () => {} })); +vi.mock('@/app/reader/hooks/useFileSync', () => ({ useFileSync: () => {} })); +vi.mock('@/app/reader/hooks/useTextTranslation', () => ({ useTextTranslation: () => {} })); +vi.mock('@/hooks/useBackgroundTexture', () => ({ + useBackgroundTexture: () => ({ applyBackgroundTexture: vi.fn() }), +})); +vi.mock('@/hooks/useAutoFocus', () => ({ useAutoFocus: () => {} })); +vi.mock('@/hooks/useEinkMode', () => ({ useEinkMode: () => ({ applyEinkMode: () => {} }) })); +vi.mock('@/hooks/useUICSS', () => ({ useUICSS: () => {} })); +vi.mock('@/hooks/useDiscordPresence', () => ({ useDiscordPresence: () => {} })); +vi.mock('@/app/reader/hooks/bookOrbitProgressProvider', () => ({ bookOrbitProgressProvider: {} })); +vi.mock('@/app/reader/components/paragraph', () => ({ ParagraphControl: () => null })); +vi.mock('@/app/reader/components/BrightnessOverlay', () => ({ default: () => null })); +vi.mock('@/app/reader/components/ImageViewer', () => ({ default: () => null })); +vi.mock('@/app/reader/components/TableViewer', () => ({ default: () => null })); + +// #6710: the book document outlives the reader (bookDataStore caches it for +// the next open), so a listener the view adds to its transformTarget keeps +// that closed view, its iframes and their listeners alive, one more per open. +function setup() { + const transformTarget = new EventTarget(); + const live = new Set(); + const add = transformTarget.addEventListener.bind(transformTarget); + const remove = transformTarget.removeEventListener.bind(transformTarget); + transformTarget.addEventListener = (type, listener, options) => { + if (listener) live.add(listener); + add(type, listener, options); + }; + transformTarget.removeEventListener = (type, listener, options) => { + if (listener) live.delete(listener); + remove(type, listener, options); + }; + const props = { + bookKey: 'test-book', + bookDoc: { metadata: {}, transformTarget }, + config: {}, + gridInsets: { top: 0, right: 0, bottom: 0, left: 0 }, + contentInsets: { top: 0, right: 0, bottom: 0, left: 0 }, + } as unknown as ComponentProps; + return { live, props }; +} + +it('removes its transformTarget listeners when the reader closes', async () => { + openGate.pending = null; + const { live, props } = setup(); + const { unmount } = render(); + await waitFor(() => expect(live.size).toBeGreaterThan(0)); + unmount(); + expect(live.size).toBe(0); +}); + +it('leaves no transformTarget listeners when the reader closes while the book is opening', async () => { + let finishOpen = () => {}; + openGate.pending = new Promise((resolve) => { + finishOpen = resolve; + }); + openGate.started = false; + const { live, props } = setup(); + const { unmount } = render(); + await waitFor(() => expect(openGate.started).toBe(true)); + unmount(); + finishOpen(); + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(live.size).toBe(0); +}); diff --git a/apps/readest-app/src/__tests__/document/paginator-transform-listener.browser.test.ts b/apps/readest-app/src/__tests__/document/paginator-transform-listener.browser.test.ts new file mode 100644 index 000000000..f0c30a9e4 --- /dev/null +++ b/apps/readest-app/src/__tests__/document/paginator-transform-listener.browser.test.ts @@ -0,0 +1,35 @@ +import { describe, it, expect } from 'vitest'; +import type { Renderer } from '@/types/view'; + +// #6710: the reader keeps a book document cached after it closes, so a +// listener the paginator leaves on its transformTarget keeps the destroyed +// paginator, its iframes and their listeners alive, one more per open. +describe('paginator transformTarget listener', () => { + it('is removed when the paginator is destroyed', async () => { + await import('foliate-js/paginator.js'); + const transformTarget = new EventTarget(); + const live = new Set(); + const add = transformTarget.addEventListener.bind(transformTarget); + const remove = transformTarget.removeEventListener.bind(transformTarget); + transformTarget.addEventListener = (type, listener, options) => { + if (listener) live.add(listener); + add(type, listener, options); + }; + transformTarget.removeEventListener = (type, listener, options) => { + if (listener) live.delete(listener); + remove(type, listener, options); + }; + + const paginator = document.createElement('foliate-paginator') as Renderer & { + open: (book: unknown) => void; + destroy: () => void; + }; + document.body.append(paginator); + paginator.open({ sections: [], transformTarget }); + expect(live.size).toBe(1); + + paginator.destroy(); + paginator.remove(); + expect(live.size).toBe(0); + }); +}); diff --git a/apps/readest-app/src/app/reader/components/FoliateViewer.tsx b/apps/readest-app/src/app/reader/components/FoliateViewer.tsx index be3c90c1d..557e6ac32 100644 --- a/apps/readest-app/src/app/reader/components/FoliateViewer.tsx +++ b/apps/readest-app/src/app/reader/components/FoliateViewer.tsx @@ -178,9 +178,18 @@ const FoliateViewer: React.FC<{ const { registerSpeedListeners, overlayVisible: speedOverlayVisible } = useAutoScrollSpeedGesture(autoScroll); + // The book document outlives this view: bookDataStore keeps it for the next + // open, so listeners left on its transformTarget would keep the closed view, + // its iframes and their listeners alive, one more set per open (#6710). + const removeTransformListenersRef = useRef<(() => void) | null>(null); + const unmountedRef = useRef(false); + // A pending anti-flash timer must not fire setNavigating on an unmounted component. useEffect(() => { + unmountedRef.current = false; return () => { + unmountedRef.current = true; + removeTransformListenersRef.current?.(); if (navSpinnerTimerRef.current) clearTimeout(navSpinnerTimerRef.current); if (librarySearchHighlightTimerRef.current) { clearTimeout(librarySearchHighlightTimerRef.current); @@ -763,7 +772,7 @@ const FoliateViewer: React.FC<{ const { book } = view; - book.transformTarget?.addEventListener('load', async (event: Event) => { + const handleTransformLoad = async (event: Event) => { const { detail } = event as CustomEvent<{ isScript: boolean; type: string; @@ -789,12 +798,20 @@ const FoliateViewer: React.FC<{ } }); } - }); + }; const viewWidth = appService?.isMobile ? screen.width : window.innerWidth; const viewHeight = appService?.isMobile ? screen.height : window.innerHeight; const width = viewWidth - insets.left - insets.right; const height = viewHeight - insets.top - insets.bottom; - book.transformTarget?.addEventListener('data', getDocTransformHandler({ width, height })); + const handleTransformData = getDocTransformHandler({ width, height }); + book.transformTarget?.addEventListener('load', handleTransformLoad); + book.transformTarget?.addEventListener('data', handleTransformData); + removeTransformListenersRef.current = () => { + book.transformTarget?.removeEventListener('load', handleTransformLoad); + book.transformTarget?.removeEventListener('data', handleTransformData); + }; + // The reader can close while view.open() is pending; its cleanup has run. + if (unmountedRef.current) removeTransformListenersRef.current(); view.renderer.setStyles?.(getStyles(viewSettings, undefined, getLoadedFonts())); applyTranslationStyle(viewSettings); diff --git a/packages/foliate-js b/packages/foliate-js index 3d7941954..a1319f4b3 160000 --- a/packages/foliate-js +++ b/packages/foliate-js @@ -1 +1 @@ -Subproject commit 3d7941954a98b136316a795e8200d60c9643b141 +Subproject commit a1319f4b3535bd9fff14b01078409b0f645edd4e