diff --git a/src/main/durable-file-write-syscall-proof.test.ts b/src/main/durable-file-write-syscall-proof.test.ts new file mode 100644 index 000000000..bf18d6c00 --- /dev/null +++ b/src/main/durable-file-write-syscall-proof.test.ts @@ -0,0 +1,67 @@ +// Empirical proof that the durable write fsyncs the file, and the directory where the platform +// allows it. Counted at the module boundary rather than inferred from reading the implementation. +import { closeSync, fsyncSync, mkdtempSync, openSync, readFileSync, rmSync } from 'node:fs' +import type * as NodeFs from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { expect, it, vi } from 'vitest' + +/** Why the rename is recorded too: an fsync moved after the rename still fsyncs a file, so a + * fsync-only log reads identically for the correct and the broken order. The rename is the boundary + * the ordering is defined against, so it has to appear in the same sequence. */ +const syscalls: ('fsync:file' | 'fsync:directory' | 'rename')[] = [] + +vi.mock('node:fs', async () => { + const actual = await vi.importActual('node:fs') + return { + ...actual, + fsyncSync: (fd: number) => { + syscalls.push(actual.fstatSync(fd).isDirectory() ? 'fsync:directory' : 'fsync:file') + return actual.fsyncSync(fd) + }, + renameSync: (from: NodeFs.PathLike, to: NodeFs.PathLike) => { + syscalls.push('rename') + return actual.renameSync(from, to) + } + } +}) + +/** Windows cannot open a directory for fsync, and some filesystems reject it; probe rather than + * assume, so the expectation tracks the real platform instead of a hardcoded OS list. */ +function directoryFsyncSupported(directory: string): boolean { + let fd: number | null = null + try { + fd = openSync(directory, 'r') + fsyncSync(fd) + return true + } catch { + return false + } finally { + if (fd !== null) { + try { + closeSync(fd) + } catch { + // Nothing actionable in a probe. + } + } + } +} + +it('fsyncs the file before rename, and the directory after where supported', async () => { + const { writeFileDurableSync } = await import('./durable-file-write') + const dir = mkdtempSync(join(tmpdir(), 'orca-fsync-')) + try { + const supported = directoryFsyncSupported(dir) + syscalls.length = 0 // Discard the probe's own fsync. + const target = join(dir, 'x.json') + writeFileDurableSync(`${target}.tmp`, target, '{"ok":1}') + expect(readFileSync(target, 'utf-8')).toBe('{"ok":1}') + // The data fsync must precede the rename: that ordering is the entire fix. Publishing the name + // first is what lets a crash expose a stale or zero-length file. + expect(syscalls).toEqual( + supported ? ['fsync:file', 'rename', 'fsync:directory'] : ['fsync:file', 'rename'] + ) + } finally { + rmSync(dir, { recursive: true, force: true }) + } +}) diff --git a/src/main/durable-file-write.test.ts b/src/main/durable-file-write.test.ts new file mode 100644 index 000000000..c2a44bf38 --- /dev/null +++ b/src/main/durable-file-write.test.ts @@ -0,0 +1,87 @@ +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { writeFileDurable, writeFileDurableSync } from './durable-file-write' + +describe('durable file write', () => { + let dir: string + + beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), 'orca-durable-')) + }) + + afterEach(() => { + rmSync(dir, { recursive: true, force: true }) + }) + + for (const [label, write] of [ + ['async', (t: string, f: string, p: string) => writeFileDurable(t, f, p)], + [ + 'sync', + (t: string, f: string, p: string) => { + writeFileDurableSync(t, f, p) + return Promise.resolve() + } + ] + ] as const) { + describe(label, () => { + it('publishes the payload at the final path', async () => { + const final = join(dir, 'state.json') + await write(`${final}.tmp`, final, '{"a":1}') + expect(readFileSync(final, 'utf-8')).toBe('{"a":1}') + }) + + it('replaces existing content atomically', async () => { + const final = join(dir, 'state.json') + writeFileSync(final, 'stale', 'utf-8') + await write(`${final}.tmp`, final, 'fresh') + expect(readFileSync(final, 'utf-8')).toBe('fresh') + }) + + it('leaves no temp file behind on success', async () => { + const final = join(dir, 'state.json') + const tmp = `${final}.tmp` + await write(tmp, final, 'x') + expect(() => readFileSync(tmp, 'utf-8')).toThrow() + }) + + it('round-trips a multi-megabyte payload without truncation', async () => { + // Why: the real orca-data.json is large; a partial fsync would surface here. + const final = join(dir, 'big.json') + const payload = JSON.stringify({ blob: 'x'.repeat(4 * 1024 * 1024) }) + await write(`${final}.tmp`, final, payload) + expect(readFileSync(final, 'utf-8')).toHaveLength(payload.length) + }) + + it('preserves exact bytes for multibyte and escape-sensitive content', async () => { + const final = join(dir, 'utf8.json') + const payload = JSON.stringify({ s: 'emoji 🚀 + 日本語 + \u0000 + "quotes"' }) + await write(`${final}.tmp`, final, payload) + expect(readFileSync(final, 'utf-8')).toBe(payload) + }) + + it('surfaces an unwritable temp path instead of silently succeeding', async () => { + const final = join(dir, 'state.json') + const tmp = join(dir, 'missing-subdir', 'state.json.tmp') + // The sync variant throws synchronously and the async one rejects; both must fail loudly + // and neither may publish a partial file. + let failed = false + try { + await write(tmp, final, 'x') + } catch { + failed = true + } + expect(failed).toBe(true) + expect(() => readFileSync(final, 'utf-8')).toThrow() + }) + }) + } + + it('keeps the last writer when async and sync paths target one file', async () => { + const final = join(dir, 'state.json') + await writeFileDurable(`${final}.a.tmp`, final, 'from-async') + writeFileDurableSync(`${final}.b.tmp`, final, 'from-sync') + expect(readFileSync(final, 'utf-8')).toBe('from-sync') + }) +}) diff --git a/src/main/durable-file-write.ts b/src/main/durable-file-write.ts new file mode 100644 index 000000000..7f827ba8a --- /dev/null +++ b/src/main/durable-file-write.ts @@ -0,0 +1,83 @@ +// Why: rename() is atomic for readers but not durable. Without fsync on the file and its directory, +// a power loss after a successful rename can leave the old contents, or an empty inode — the same +// empty-file symptom as issue #1158, from a different cause. The .bak ring recovers it at up to an +// hour's loss; fsync stops it from happening. + +import { closeSync, fsyncSync, openSync, renameSync, writeFileSync } from 'node:fs' +import { open, rename } from 'node:fs/promises' +import { dirname } from 'node:path' + +/** + * fsync a directory so a rename within it is durable. Best-effort by design: Windows cannot open a + * directory for fsync, and some filesystems reject it. The file fsync above it is the load-bearing + * part; this closes the "rename recorded but not persisted" window where the platform allows it. + */ +async function syncDirectory(directory: string): Promise { + let handle: Awaited> | null = null + try { + handle = await open(directory, 'r') + await handle.sync() + } catch { + // Expected on Windows and on filesystems without directory fsync. + } finally { + await handle?.close().catch(() => {}) + } +} + +function syncDirectorySync(directory: string): void { + let fd: number | null = null + try { + fd = openSync(directory, 'r') + fsyncSync(fd) + } catch { + // Same platform caveats as syncDirectory. + } finally { + if (fd !== null) { + try { + closeSync(fd) + } catch { + // Nothing actionable; the fsync already happened or the open failed. + } + } + } +} + +/** + * Rename and then fsync the containing directory. For callers that already fsynced the temp file + * themselves and need the rename made durable. + */ +export async function renameDurable(tmpPath: string, finalPath: string): Promise { + await rename(tmpPath, finalPath) + await syncDirectory(dirname(finalPath)) +} + +/** Write `payload` to `tmpPath`, fsync it, then rename onto `finalPath` and fsync the directory. */ +export async function writeFileDurable( + tmpPath: string, + finalPath: string, + payload: string +): Promise { + const handle = await open(tmpPath, 'w') + try { + await handle.writeFile(payload, 'utf-8') + // Why: fsync BEFORE rename. A rename that lands first can expose a zero-length file. + await handle.sync() + } finally { + await handle.close() + } + await rename(tmpPath, finalPath) + await syncDirectory(dirname(finalPath)) +} + +/** Synchronous counterpart for quit and crash paths that cannot await. */ +export function writeFileDurableSync(tmpPath: string, finalPath: string, payload: string): void { + writeFileSync(tmpPath, payload, 'utf-8') + const fd = openSync(tmpPath, 'r+') + try { + fsyncSync(fd) + } finally { + closeSync(fd) + } + renameSync(tmpPath, finalPath) + syncDirectorySync(dirname(finalPath)) +} diff --git a/src/main/persistence.ts b/src/main/persistence.ts index b26c68a9a..48495b7a1 100644 --- a/src/main/persistence.ts +++ b/src/main/persistence.ts @@ -11,7 +11,8 @@ import { statSync, realpathSync } from 'node:fs' -import { writeFile, rename, mkdir, rm, copyFile } from 'node:fs/promises' +import { rename, mkdir, rm, copyFile, open } from 'node:fs/promises' +import { renameDurable, writeFileDurableSync } from './durable-file-write' import { join, dirname, isAbsolute, resolve, sep } from 'node:path' import { homedir } from 'node:os' import { createHash, randomUUID } from 'node:crypto' @@ -3686,12 +3687,19 @@ export class Store { // Why: on any write/rename failure, remove the tmp file so it doesn't leave a multi-MB orphan. let renamed = false try { - await writeFile(tmpFile, payload, 'utf-8') + // Why: fsync before rename, then fsync the directory; see writeFileDurable. + const handle = await open(tmpFile, 'w') + try { + await handle.writeFile(payload, 'utf-8') + await handle.sync() + } finally { + await handle.close() + } // Why: if flush() bumped writeGeneration mid-write, it already wrote fresher state; don't overwrite it. if (this.writeGeneration !== gen) { return } - await rename(tmpFile, dataFile) + await renameDurable(tmpFile, dataFile) renamed = true // Why re-check gen: a sync flush during the rename await may have written fresher state; don't record a stale hash over it. if (this.writeGeneration === gen) { @@ -3732,8 +3740,9 @@ export class Store { // Why: on any write/rename failure, remove the tmp file so shutdown crashes don't leak orphans. let renamed = false try { - writeFileSync(tmpFile, payload, 'utf-8') - renameSync(tmpFile, dataFile) + // Why: fsync the temp file and the directory; a bare rename can survive as stale or empty + // content after power loss, losing projects/tabs back to the newest usable .bak slot. + writeFileDurableSync(tmpFile, dataFile, payload) renamed = true this.lastWrittenStateHash = stateHash } finally {