History

fix(local): survive Windows libSQL handle linger during v1->v2 migration (#1209)

The v1->v2 data migration ran fsync/rename/remove on libSQL SQLite files whose native OS handle lingers past close() on Windows, crashing the daemon at boot with a fatal "Unknown error". POSIX never hits this, so it only reproduced on Windows (the v1.5.23 desktop/CLI startup regression). - fsyncFileIfExists opens read-write, since Windows FlushFileBuffers needs a writable handle (a read-only fd throws EPERM), and treats fsync as best-effort durability hardening. - remove of the source/staging file sets now retries EBUSY/EPERM the same way rename already did. - rename/remove retries force a GC pass first so libSQL's native finalizer releases the handle before the next attempt. Verified on a Windows VM: a seeded v1 database now migrates to v2 and the daemon announces ready.

Rhys Sullivan committed 4 months ago

Commit ffa4f701 parent b1d73bf2 files · +62 −14Browse files

Unified · Split

2 files
  1. .changeset/windows-migration-handle-release.md +5 −0
  2. apps/local/src/db/v1-v2-migration.ts +57 −14
.changeset/windows-migration-handle-release.md+5−0
@@ -0,0 +1,5 @@
1+---
2+"executor": patch
3+---
4+
5+Fix the desktop and CLI daemon crashing on first launch on Windows when a v1 local database is present. The v1 to v2 data migration performed file operations (fsync, rename, remove) on libSQL SQLite files whose native OS handles linger after close() on Windows, surfacing as a fatal "Unknown error" (EPERM on fsync of a read-only handle, EBUSY on rename/remove of just-closed files). POSIX is unaffected, so this only reproduced on Windows. The migration now opens files read-write for fsync (treating it as best-effort), retries removes the same way renames were already retried, and forces a GC pass on each retry so libSQL's native finalizer releases the handle before the next attempt. Fixes the v1.5.23 Windows startup regression.
apps/local/src/db/v1-v2-migration.ts+57−14
@@ -1003,10 +1003,19 @@ const insertPlan = async ( · 1002 unmodified lines
10031003 }
10041004 };
10051005
1006+// libSQL releases its OS file handle in a native finalizer, which on Windows
1007+// only runs on garbage collection and so lags client.close(). Forcing a GC
1008+// runs the finalizer, releasing the handle so a follow-up rename/rm succeeds.
1009+// No-op outside the Bun runtime (e.g. vitest on Node), where this is unneeded.
1010+const nudgeNativeHandleRelease = (): void => {
1011+ const bun = (globalThis as { Bun?: { gc?: (sync: boolean) => void } }).Bun;
1012+ bun?.gc?.(true);
1013+};
1014+
10061015 // Windows reports EBUSY/EPERM on rename for a short window after a SQLite
10071016 // handle closes (handle release lags the close call, and antivirus scanners
10081017 // briefly lock the file). POSIX renames never hit this — retry with a short
1009−// backoff instead of failing the whole migration.
1018+// backoff, forcing handle release first, instead of failing the whole migration.
10101019 const renameWithRetry = async (source: string, target: string): Promise<void> => {
10111020 // ~8s total. 1.9s was not enough on Windows: libSQL's native handle and
10121021 // antivirus scans hold the freshly-written db past close() (observed as an
@@ -1019,16 +1028,51 @@ const renameWithRetry = async (source: string, target: string): Promise<void> => · 6 unmodified lines
10191028 } catch (cause) {
10201029 const code = (cause as NodeJS.ErrnoException).code;
10211030 if ((code !== "EBUSY" && code !== "EPERM") || attempt >= delaysMs.length) throw cause;
1031+ nudgeNativeHandleRelease();
1032+ await new Promise((resolveDelay) => setTimeout(resolveDelay, delaysMs[attempt]));
1033+ }
1034+ }
1035+};
1036+
1037+// Same lingering-handle window as renameWithRetry: on Windows a libSQL file's
1038+// native handle (and antivirus locks) can briefly outlive close(), so removing
1039+// a just-closed source/staging db throws EBUSY/EPERM. POSIX never hits this.
1040+const rmWithRetry = async (path: string): Promise<void> => {
1041+ const delaysMs = [50, 100, 250, 500, 1000, 2000, 4000];
1042+ for (let attempt = 0; ; attempt++) {
1043+ try {
1044+ fs.rmSync(path, { force: true });
1045+ return;
1046+ } catch (cause) {
1047+ const code = (cause as NodeJS.ErrnoException).code;
1048+ if ((code !== "EBUSY" && code !== "EPERM") || attempt >= delaysMs.length) throw cause;
1049+ nudgeNativeHandleRelease();
10221050 await new Promise((resolveDelay) => setTimeout(resolveDelay, delaysMs[attempt]));
10231051 }
10241052 }
10251053 };
10261054
1055+const removeSqliteFileSetWithRetry = async (path: string): Promise<void> => {
1056+ for (const suffix of fileSetSuffixes) await rmWithRetry(`${path}${suffix}`);
1057+};
1058+
10271059 const fsyncFileIfExists = (path: string): void => {
10281060 if (!fs.existsSync(path)) return;
1029− const fd = fs.openSync(path, "r");
1061+ // Windows FlushFileBuffers requires a writable handle, so fsync on a
1062+ // read-only ("r") fd throws EPERM and would crash the v1->v2 migration at
1063+ // boot. Open read-write for the flush, and treat fsync as best-effort
1064+ // durability hardening (the rename/copy already wrote the bytes) so a
1065+ // platform or filesystem that refuses the flush does not fail the migration.
1066+ let fd: number;
1067+ try {
1068+ fd = fs.openSync(path, "r+");
1069+ } catch {
1070+ return;
1071+ }
10301072 try {
10311073 fs.fsyncSync(fd);
1074+ } catch {
1075+ // best-effort: some platforms/filesystems refuse fsync on certain handles
10321076 } finally {
10331077 fs.closeSync(fd);
10341078 }
@@ -1060,7 +1104,7 @@ const moveSqliteFileSet = async (source: string, target: string): Promise<void> · 25 unmodified lines
10601104 await renameWithRetry(`${source}${suffix}`, `${target}${suffix}`);
10611105 fsyncDirectory(dirname(target));
10621106 } else {
1063− fs.rmSync(`${target}${suffix}`, { force: true });
1107+ await rmWithRetry(`${target}${suffix}`);
10641108 }
10651109 }
10661110 };
@@ -1071,7 +1115,7 @@ const moveExistingSqliteFileSet = async (source: string, target: string): Promis · 4 unmodified lines
10711115 if (!fs.existsSync(from)) continue;
10721116 const to = `${target}${suffix}`;
10731117 if (fs.existsSync(to)) {
1074− fs.rmSync(from, { force: true });
1118+ await rmWithRetry(from);
10751119 } else {
10761120 await renameWithRetry(from, to);
10771121 fsyncDirectory(dirname(to));
@@ -1413,8 +1457,8 @@ const completeJournaledFlip = async ( · 335 unmodified lines
14131457 await pauseMigrationForTest("staging-consumed");
14141458 }
14151459 await writeMigrationJournal(journalPath, { ...journal, phase: "committed" });
1416− removeSqliteFileSet(journal.normalizedSource);
1417− removeSqliteFileSet(journal.staging);
1460+ await removeSqliteFileSetWithRetry(journal.normalizedSource);
1461+ await removeSqliteFileSetWithRetry(journal.staging);
14181462 await cleanupSecretBackups(journal);
14191463 removeMigrationJournal(journalPath);
14201464 };
@@ -1435,8 +1479,8 @@ const recoverV1V2Migration = async (sqlitePath: string): Promise<MigrationRecove · 14 unmodified lines
14351479
14361480 const { journal } = journalRead;
14371481 if (journal.phase === "building") {
1438− removeSqliteFileSet(journal.normalizedSource);
1439− removeSqliteFileSet(journal.staging);
1482+ await removeSqliteFileSetWithRetry(journal.normalizedSource);
1483+ await removeSqliteFileSetWithRetry(journal.staging);
14401484 await restoreAuthFromJournal(journal);
14411485 await cleanupSecretBackups(journal);
14421486 removeMigrationJournal(journalPath);
@@ -1448,8 +1492,8 @@ const recoverV1V2Migration = async (sqlitePath: string): Promise<MigrationRecove · 5 unmodified lines
14481492 return "recovered";
14491493 }
14501494
1451− removeSqliteFileSet(journal.normalizedSource);
1452− removeSqliteFileSet(journal.staging);
1495+ await removeSqliteFileSetWithRetry(journal.normalizedSource);
1496+ await removeSqliteFileSetWithRetry(journal.staging);
14531497 await cleanupSecretBackups(journal);
14541498 removeMigrationJournal(journalPath);
14551499 return "recovered";
@@ -1573,8 +1617,7 @@ export const migrateLocalV1ToV2IfNeeded = async ( · 117 unmodified lines
15731617 // WAL/SHM sidecars before journaling the staged DB as complete; otherwise a
15741618 // crash mid-sidecar move would make staging appear present after the base DB
15751619 // had already been installed.
1576− for (const suffix of ["-wal", "-shm"] as const)
1577− fs.rmSync(`${stagingPath}${suffix}`, { force: true });
1620+ for (const suffix of ["-wal", "-shm"] as const) await rmWithRetry(`${stagingPath}${suffix}`);
15781621 fsyncDirectory(dirname(stagingPath));
15791622 await writeMigrationJournal(journalPath, { ...journal, phase: "built" });
15801623 await pauseMigrationForTest("built");
@@ -1598,8 +1641,8 @@ export const migrateLocalV1ToV2IfNeeded = async ( · 17 unmodified lines
15981641 await restoreAuthFromJournal(journal);
15991642 await cleanupSecretBackups(journal);
16001643 }
1601− if (normalizedSourcePath) removeSqliteFileSet(normalizedSourcePath);
1602− if (stagingPath) removeSqliteFileSet(stagingPath);
1644+ if (normalizedSourcePath) await removeSqliteFileSetWithRetry(normalizedSourcePath);
1645+ if (stagingPath) await removeSqliteFileSetWithRetry(stagingPath);
16031646 removeMigrationJournal(journalPath);
16041647 }
16051648 throw cause;