diff --git a/.changeset/relaxed-sync-latch-and-close-drain.md b/.changeset/relaxed-sync-latch-and-close-drain.md new file mode 100644 index 000000000..c6ebfee26 --- /dev/null +++ b/.changeset/relaxed-sync-latch-and-close-drain.md @@ -0,0 +1,8 @@ +--- +'@electric-sql/pglite': patch +--- + +Fix two ways a `relaxedDurability` database could silently lose persistence: + +- A failed fire-and-forget background sync was an unhandled promise rejection and the database kept running without ever persisting again. The first failure is now latched and thrown by the next query or explicit `syncToFs()` call. +- `close()` could close the filesystem while a fire-and-forget sync was still running against it — on IdbFs this opened a transaction on an already-closing IndexedDB connection (an uncaught `InvalidStateError` from inside an Emscripten callback, reproduced on Chrome and Firefox) and the tail writes could be dropped. `close()` now drains any in-flight sync and performs a final strict sync before closing the filesystem, and reports a final-sync failure after releasing filesystem resources. diff --git a/packages/pglite/src/pglite.ts b/packages/pglite/src/pglite.ts index e23ac32c4..074d039b6 100644 --- a/packages/pglite/src/pglite.ts +++ b/packages/pglite/src/pglite.ts @@ -97,6 +97,7 @@ export class PGlite #listenMutex = new Mutex() #fsSyncMutex = new Mutex() #fsSyncScheduled = false + #fsSyncFailure?: { error: unknown } readonly debug: DebugLevel = 0 @@ -812,7 +813,24 @@ export class PGlite this.mod!.removeFunction(this.#pglite_socket_write) } - // Close the filesystem + // Drain any in-flight relaxed-durability sync and perform a final strict + // sync before closing the filesystem. With relaxedDurability the last + // syncToFs() is fire-and-forget, so a sync can still be running against + // the filesystem here; the mutex serialises with it (waiting it out) and + // the strict syncToFs(false) guarantees the tail writes persist. Without + // this, on IdbFs an in-flight sync could open a transaction on an + // already-closing IDBDatabase connection (an uncaught InvalidStateError + // from inside an Emscripten callback) and the tail writes could be + // dropped. + let finalSyncError: { error: unknown } | undefined + try { + await this.#fsSyncMutex.runExclusive(() => this.fs!.syncToFs(false)) + } catch (error) { + finalSyncError = { error } + } + + // Close the filesystem even if the final sync failed so that any + // exclusive storage ownership is always released. await this.fs!.closeFs() this.#closed = true @@ -830,6 +848,10 @@ export class PGlite this.#log('Error when exiting', e.toString()) } } + + if (finalSyncError) { + throw finalSyncError.error + } } /** @@ -1130,6 +1152,12 @@ export class PGlite * run after every query to ensure that the filesystem is synced. */ async syncToFs() { + if (this.#fsSyncFailure) { + // A previous background sync failed, so the database is no longer being + // persisted; surface the failure instead of continuing without + // durability. + throw this.#fsSyncFailure.error + } if (this.#fsSyncScheduled) { return } @@ -1143,8 +1171,18 @@ export class PGlite } if (this.#relaxedDurability) { - doSync() + // The sync is intentionally not awaited, but its failure must not + // become an unhandled rejection: latch the first error so the next + // query or explicit syncToFs() call throws it. + void doSync().catch((error) => { + this.#fsSyncFailure ??= { error } + }) } else { + // Awaited: the failure already reaches the caller, so it is NOT + // latched — the filesystem stays reachable and a stateful filesystem + // implementation decides for itself what later calls should throw. + // The latch exists only for the detached path above, which has no + // other way to report. await doSync() } } diff --git a/packages/pglite/tests/close-drain.test.ts b/packages/pglite/tests/close-drain.test.ts new file mode 100644 index 000000000..a54fe331e --- /dev/null +++ b/packages/pglite/tests/close-drain.test.ts @@ -0,0 +1,65 @@ +import { describe, expect, it } from 'vitest' +import { MemoryFS, PGlite } from '../dist/index.js' + +class GatedFS extends MemoryFS { + events: string[] = [] + #gate?: Promise + #releaseGate: () => void = () => {} + + gateNextRelaxedSync(): () => void { + this.#gate = new Promise((resolve) => { + this.#releaseGate = resolve + }) + return () => this.#releaseGate() + } + + override async syncToFs(relaxedDurability?: boolean): Promise { + const kind = relaxedDurability ? 'relaxed' : 'strict' + this.events.push(`sync:${kind}:start`) + if (relaxedDurability && this.#gate) { + const gate = this.#gate + this.#gate = undefined + await gate + } + await super.syncToFs(relaxedDurability) + this.events.push(`sync:${kind}:end`) + } + + override async closeFs(): Promise { + this.events.push('closeFs') + await super.closeFs() + } +} + +describe('close with an in-flight relaxed sync', () => { + it('drains the sync and performs a final strict sync before closing the filesystem', async () => { + const fs = new GatedFS() + const pg = await PGlite.create({ fs, relaxedDurability: true }) + + // Let any detached sync scheduled during initialization settle before + // instrumenting the run we assert on. + await new Promise((resolve) => setTimeout(resolve, 20)) + fs.events.length = 0 + + const release = fs.gateNextRelaxedSync() + await pg.exec('SELECT 1') + + const closePromise = pg.close() + await new Promise((resolve) => setTimeout(resolve, 50)) + // The gated relaxed sync is still in flight; close() must not have + // reached the filesystem close yet. + expect(fs.events).not.toContain('closeFs') + + release() + await closePromise + + const closeFsAt = fs.events.indexOf('closeFs') + const relaxedEndAt = fs.events.indexOf('sync:relaxed:end') + const strictEndAt = fs.events.lastIndexOf('sync:strict:end') + // The in-flight relaxed sync completed, a final strict sync followed it, + // and only then was the filesystem closed. + expect(relaxedEndAt).toBeGreaterThanOrEqual(0) + expect(strictEndAt).toBeGreaterThan(relaxedEndAt) + expect(closeFsAt).toBeGreaterThan(strictEndAt) + }) +}) diff --git a/packages/pglite/tests/relaxed-sync-failure.test.ts b/packages/pglite/tests/relaxed-sync-failure.test.ts new file mode 100644 index 000000000..27c9b01a4 --- /dev/null +++ b/packages/pglite/tests/relaxed-sync-failure.test.ts @@ -0,0 +1,86 @@ +import { describe, expect, it } from 'vitest' +import { MemoryFS, PGlite } from '../dist/index.js' + +class RejectingFS extends MemoryFS { + readonly failure = new Error('forced detached sync failure') + readonly failureObserved: Promise + #failNextRelaxedSync = false + #resolveFailureObserved: () => void = () => {} + + constructor() { + super() + this.failureObserved = new Promise((resolve) => { + this.#resolveFailureObserved = resolve + }) + } + + failNextRelaxedSync(): void { + this.#failNextRelaxedSync = true + } + + override async syncToFs(relaxedDurability?: boolean): Promise { + if (relaxedDurability && this.#failNextRelaxedSync) { + this.#failNextRelaxedSync = false + this.#resolveFailureObserved() + throw this.failure + } + await super.syncToFs(relaxedDurability) + } +} + +class AwaitedFailureFS extends MemoryFS { + readonly failure = new Error('forced awaited sync failure') + syncCalls = 0 + #failNextSync = false + + failNextSync(): void { + this.#failNextSync = true + } + + override async syncToFs(relaxedDurability?: boolean): Promise { + this.syncCalls += 1 + if (this.#failNextSync) { + this.#failNextSync = false + throw this.failure + } + await super.syncToFs(relaxedDurability) + } +} + +describe('relaxed-durability filesystem sync failure', () => { + it('latches a detached relaxed rejection for the next public query and sync', async () => { + const fs = new RejectingFS() + const pg = await PGlite.create({ fs, relaxedDurability: true }) + fs.failNextRelaxedSync() + + await pg.exec('SELECT 1') + await fs.failureObserved + + await expect(pg.exec('SELECT 2')).rejects.toBe(fs.failure) + await expect(pg.syncToFs()).rejects.toBe(fs.failure) + + // close() calls the filesystem directly rather than going through the + // latched public syncToFs(), so a store whose syncs recovered can still + // persist its final state. + await pg.close() + }) + + it('does NOT latch an awaited failure — the caller gets it once and the filesystem stays reachable', async () => { + // With relaxedDurability: false the rejection already reaches the caller, + // so latching would only REPLAY it at the next syncToFs() and shadow a + // stateful filesystem's own failure policy (a custom fs may poison itself + // and must be the one deciding what later calls throw). + const fs = new AwaitedFailureFS() + const pg = await PGlite.create({ fs, relaxedDurability: false }) + await pg.exec('CREATE TABLE t (v int)') + + fs.failNextSync() + await expect(pg.exec('INSERT INTO t VALUES (1)')).rejects.toBe(fs.failure) + + const callsAfterFailure = fs.syncCalls + // The next query must reach the filesystem again — not a replayed latch. + await pg.exec('SELECT 1') + expect(fs.syncCalls).toBeGreaterThan(callsAfterFailure) + await pg.close() + }) +})