From ec6781fe76fcf61402f7f44c9a90b90bd8a1300e Mon Sep 17 00:00:00 2001 From: Huang Xin Date: Thu, 9 Jul 2026 03:37:02 +0900 Subject: [PATCH] fix: media-session teardown race + page_stat view migration idempotency (#5019) * fix(tts): guard media session against teardown during bind() TTSMediaBridge.bind() resolves #mediaSession, then awaits the cover fetch and setActive. A concurrent unbind() (a stop during startup) nulls #mediaSession mid-flight, so the subsequent this.#mediaSession.setActive/.updateMetadata threw "Cannot read properties of null" (READEST-1A). Capture the session in a local for the awaited calls, and bail before wiring action handlers if it was torn down. Adds a regression test forcing the Tauri branch with a mid-fetch unbind. Fixes READEST-1A Co-Authored-By: Claude Opus 4.8 (1M context) * fix(stats): make the page_stat view migration idempotent under turso turso ignores IF NOT EXISTS on CREATE VIEW (core/schema.rs discards the flag), so CREATE VIEW IF NOT EXISTS page_stat still throws "View page_stat already exists" when the view is present (a KOReader-imported statistics DB, or a partially-applied migration re-running). DROP VIEW IF EXISTS first (turso honors that) so the statistics migration stays idempotent. Editing the existing migration only affects DBs that have not applied it yet; version-tracked DBs fast-path skip. Adds a real-turso regression test. Fixes READEST-13 Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- .../services/tts-media-bridge.test.ts | 29 +++++++++++++++++++ .../__tests__/statistics/statisticsDb.test.ts | 16 ++++++++++ .../src/services/database/migrations/index.ts | 8 ++++- .../src/services/tts/ttsMediaBridge.ts | 13 +++++++-- 4 files changed, 62 insertions(+), 4 deletions(-) diff --git a/apps/readest-app/src/__tests__/services/tts-media-bridge.test.ts b/apps/readest-app/src/__tests__/services/tts-media-bridge.test.ts index f02d461aa..8ca8b9658 100644 --- a/apps/readest-app/src/__tests__/services/tts-media-bridge.test.ts +++ b/apps/readest-app/src/__tests__/services/tts-media-bridge.test.ts @@ -5,6 +5,8 @@ vi.mock('@/utils/image', () => ({ })); import { TTSMediaBridge } from '@/services/tts/ttsMediaBridge'; +import { fetchImageAsBase64 } from '@/utils/image'; +import { TauriMediaSession } from '@/libs/mediaSession'; import type { TTSController } from '@/services/tts/TTSController'; // A controller stand-in: EventTarget + the surface the bridge consumes. @@ -165,3 +167,30 @@ describe('TTSMediaBridge', () => { expect(bridge.isBound).toBe(true); }); }); + +describe('TTSMediaBridge bind teardown race (READEST-1A)', () => { + test('does not crash when unbound while the cover loads', async () => { + // A real TauriMediaSession instance so bind() takes the Tauri branch; its + // native methods are stubbed so nothing hits `invoke`. + const tauriSession = new TauriMediaSession(); + tauriSession.setActive = vi.fn().mockResolvedValue(undefined); + tauriSession.updateMetadata = vi.fn().mockResolvedValue(undefined); + tauriSession.setActionHandler = vi.fn(); + + const bridge = new TTSMediaBridge(() => tauriSession); + const controller = new FakeController(); + + // Tear the session down mid-flight, exactly like a stop during startup: + // #mediaSession becomes null before the awaited setActive/updateMetadata. + vi.mocked(fetchImageAsBase64).mockImplementationOnce(async () => { + bridge.unbind(); + return 'data:image/png;base64,x'; + }); + + await expect( + bridge.bind(controller as unknown as TTSController, meta({ coverImageUrl: 'cover.png' })), + ).resolves.toBeUndefined(); + // The bind aborted after teardown; no handlers were wired on a dead session. + expect(bridge.isBound).toBe(false); + }); +}); diff --git a/apps/readest-app/src/__tests__/statistics/statisticsDb.test.ts b/apps/readest-app/src/__tests__/statistics/statisticsDb.test.ts index 87802aff8..4596b76ad 100644 --- a/apps/readest-app/src/__tests__/statistics/statisticsDb.test.ts +++ b/apps/readest-app/src/__tests__/statistics/statisticsDb.test.ts @@ -32,6 +32,22 @@ describe('statistics migration', () => { expect(names).toContain('readest_stat_sync_state'); }); + it('is idempotent when the page_stat view already exists (READEST-13)', async () => { + // A DB imported from KOReader (or left by a partially-applied migration) + // already has a page_stat view but no migration record. turso ignores + // IF NOT EXISTS on CREATE VIEW, so a non-idempotent migration throws + // "View page_stat already exists" here. + const imported = await NodeDatabaseService.open(':memory:'); + await imported.execute('CREATE VIEW page_stat AS SELECT 1 AS x'); + + await expect(migrate(imported, getMigrations('statistics'))).resolves.toBeUndefined(); + + const views = await imported.select<{ name: string }>( + `SELECT name FROM sqlite_master WHERE type = 'view'`, + ); + expect(views.map((v) => v.name)).toContain('page_stat'); + }); + it('seeds the numbers helper table 1..1000', async () => { const rows = await db.select<{ c: number }>(`SELECT COUNT(*) AS c FROM numbers`); expect(rows[0]!.c).toBe(1000); diff --git a/apps/readest-app/src/services/database/migrations/index.ts b/apps/readest-app/src/services/database/migrations/index.ts index 82e793565..3e68fe7bd 100644 --- a/apps/readest-app/src/services/database/migrations/index.ts +++ b/apps/readest-app/src/services/database/migrations/index.ts @@ -83,7 +83,13 @@ const migrations: Record = { (SELECT 0 AS n UNION ALL SELECT 1 UNION ALL SELECT 2 UNION ALL SELECT 3 UNION ALL SELECT 4 UNION ALL SELECT 5 UNION ALL SELECT 6 UNION ALL SELECT 7 UNION ALL SELECT 8 UNION ALL SELECT 9) t, (SELECT 0 AS n UNION ALL SELECT 1 UNION ALL SELECT 2 UNION ALL SELECT 3 UNION ALL SELECT 4 UNION ALL SELECT 5 UNION ALL SELECT 6 UNION ALL SELECT 7 UNION ALL SELECT 8 UNION ALL SELECT 9) h; - CREATE VIEW IF NOT EXISTS page_stat AS + -- turso ignores IF NOT EXISTS on CREATE VIEW (READEST-13), so a plain + -- CREATE VIEW IF NOT EXISTS still throws "already exists" when the view + -- is present (KOReader-imported stats DB, or a partially-applied run). + -- DROP first (turso honors DROP VIEW IF EXISTS) to stay idempotent. + DROP VIEW IF EXISTS page_stat; + + CREATE VIEW page_stat AS SELECT id_book, first_page + idx - 1 AS page, start_time, duration / (last_page - first_page + 1) AS duration FROM ( SELECT id_book, page, total_pages, pages, start_time, duration, diff --git a/apps/readest-app/src/services/tts/ttsMediaBridge.ts b/apps/readest-app/src/services/tts/ttsMediaBridge.ts index 52a4405f5..55de4376e 100644 --- a/apps/readest-app/src/services/tts/ttsMediaBridge.ts +++ b/apps/readest-app/src/services/tts/ttsMediaBridge.ts @@ -107,8 +107,13 @@ export class TTSMediaBridge { this.#meta = meta; this.#mediaSession = this.#resolveMediaSession(); if (!this.#mediaSession) return; + // bind() awaits below (cover fetch, setActive), during which a concurrent + // unbind() (e.g. a stop during startup) nulls #mediaSession. Use the + // captured session for the awaited calls so they can't deref null, then + // bail before wiring handlers onto a torn-down session (READEST-1A). + const mediaSession = this.#mediaSession; - if (this.#mediaSession instanceof TauriMediaSession) { + if (mediaSession instanceof TauriMediaSession) { let artwork = '/icon.png'; try { artwork = await fetchImageAsBase64(meta.coverImageUrl || '/icon.png'); @@ -119,8 +124,8 @@ export class TTSMediaBridge { artwork = ''; } } - await this.#mediaSession.setActive({ active: true }); - await this.#mediaSession.updateMetadata({ + await mediaSession.setActive({ active: true }); + await mediaSession.updateMetadata({ title: meta.title, artist: meta.author, album: meta.title, @@ -128,6 +133,8 @@ export class TTSMediaBridge { }); } + if (this.#mediaSession !== mediaSession) return; + this.#registerActionHandlers(); this.#onSpeakMark = (e: Event) => {