diff --git a/README.md b/README.md index 7003209..8ad275c 100644 --- a/README.md +++ b/README.md @@ -1040,23 +1040,24 @@ photos newest first). `lib.subscribe({ onChange })` delivers a `LibraryChange` directory, and fails at once with an error whose `code` is `ELOCKED` while another backup of it runs. A caller can instead take the lock itself, before it opens its library, as `quak backup` does: `await lockBackupDirectory(dir)` - takes it, failing the same way, and returns the function that releases it, and - the caller passes `lockHeld: true` to the backup. It waits for a refresh as - `fresh()` does, puts every in-scope original not already at its save path - there as `photo.download()` does (and, with `includeThumbnails`, fetches - thumbnails) through the content cache, waits for an ML data fetch, and - rebuilds the on-disk backup tree with a durable failure ledger. Each file's - JSON holds its ML data and, for an image (for a live photo, its image), its - original's EXIF and XMP, with its dimensions for a JPEG only; a video's JSON - holds none of these three. A fetched original is written straight to its save - path and not into the cache, which then counts it as present; one the cache - already held is copied from there. `BackupOptions`: `downloadDirectory` (falls - back to the library's), `includeOriginals` (default `true`), - `includeThumbnails` (default `false`), `onlyAlbumNames`, `verify` (default - `false`), `onProgress`, and `lockHeld` (default `false`). See Backup layout - above for the tree it writes. Whether the backup took the lock or its caller - did, once the lock is lost the backup rejects with - `lost the lock on : ` before it starts another file. + takes it, failing the same way, and returns a `BackupLock`, whose `release()` + removes the lock unless it was lost and whose `lost` holds why it was lost. + The caller passes it to the backup as `lock`, and releases it once the backup + ends. It waits for a refresh as `fresh()` does, puts every in-scope original + not already at its save path there as `photo.download()` does (and, with + `includeThumbnails`, fetches thumbnails) through the content cache, waits for + an ML data fetch, and rebuilds the on-disk backup tree with a durable failure + ledger. Each file's JSON holds its ML data and, for an image (for a live + photo, its image), its original's EXIF and XMP, with its dimensions for a JPEG + only; a video's JSON holds none of these three. A fetched original is written + straight to its save path and not into the cache, which then counts it as + present; one the cache already held is copied from there. `BackupOptions`: + `downloadDirectory` (falls back to the library's), `includeOriginals` (default + `true`), `includeThumbnails` (default `false`), `onlyAlbumNames`, `verify` + (default `false`), `onProgress`, and `lock`. See Backup layout above for the + tree it writes. Whether the backup took the lock or its caller did, once the + lock is lost the backup rejects with `lost the lock on : ` before + it starts another file. ### Request pools @@ -1116,7 +1117,7 @@ from a very old client, is stored unchecked. - `src/library/mlsearch.ts`: `MLDataAPI`, `SimilarResult` - `src/exif.ts`: `ExifTags`, `PhotoExif` - `src/library/pools.ts`: `RequestPools`, `RequestPoolsOptions`, `BoundedPool` -- `src/backup.ts`: `BackupOptions`, `BackupResult`, `BackupError` +- `src/backup.ts`: `BackupLock`, `BackupOptions`, `BackupResult`, `BackupError` - `src/client.ts`: `Client`, `LoginOptions`, `ClientSnapshot` - `src/api/client.ts`: `ApiClient`, `ApiClientOptions`, `StreamOptions` - `src/errors.ts`: `ApiError`, `TruncatedStreamError` diff --git a/TODO.md b/TODO.md index 8418ad9..a92a337 100644 --- a/TODO.md +++ b/TODO.md @@ -27,12 +27,13 @@ declares one. - 2026-10-14: A backup whose lock is lost no longer dies with a stack trace (issue 184). `lockBackupDirectory` gives proper-lockfile an `onCompromised` - that records why the lock was lost instead of throwing from its timer. The - backup then stops before it starts another original, and before it writes - `failures.json`, with `lost the lock on DIR: REASON`, which `quak backup` - prints as one line before it exits 1. Releasing a lost lock leaves - `backup.lock` alone. Tests in `test/cli/backup-end-to-end.test.ts` remove the - lock, or put another backup's in its place, while a download is held open. + that records why the lock was lost instead of throwing from its timer, and + returns the lock, which `quak backup` passes to `lib.backup()` as `lock`. The + backup then stops before it starts another file with + `lost the lock on DIR: REASON`, which `quak backup` prints as one line before + it exits 1. Releasing a lost lock leaves `backup.lock` alone. Tests in + `test/cli/backup-end-to-end.test.ts` remove the lock, or put another backup's + in its place, while a download is held open. - 2026-10-14: `quak backup` no longer asks the server for changes every 3 seconds while it runs (issue 183). It opens its library with diff --git a/src/backup.ts b/src/backup.ts index db55675..6409151 100644 --- a/src/backup.ts +++ b/src/backup.ts @@ -1,7 +1,7 @@ // The backup command, rebuilt on the library API (issue #51). // // `lib.backup()` takes the lock in `downloadDirectory` (unless its caller -// already holds it), and fails at once when another backup of it holds the +// passes one in), and fails at once when another backup of it holds the // lock; when the lock is lost while it runs, it fails before it starts another // file. It waits for a completed refresh of the library (a failed one fails the // backup before any file is touched), then, for every file in scope, puts its @@ -93,6 +93,14 @@ import type { Collection, EnteFile, FileMetadata } from "./model/types.js"; export type ProgressCallback = (message: string) => void; +// The lock in a backup directory, as `lockBackupDirectory` returns it. +export interface BackupLock { + // Removes `backup.lock`, unless the lock was lost. + release(): Promise; + // Why the lock was lost, once it is. + lost?: Error; +} + export interface BackupOptions { // Where the backup tree lives. `lib.backup()` defaults it to the library's // download directory; `runBackup` with none throws before any network @@ -109,11 +117,11 @@ export interface BackupOptions { // bytes do not match the content hash its metadata records. Default false. verify?: boolean; onProgress?: ProgressCallback; - // The caller already holds the lock in `downloadDirectory`, taken with - // `lockBackupDirectory`, and releases it itself, so the backup does not - // take it, but still fails once that lock is lost. `quak backup` takes it + // The lock in `downloadDirectory`, when the caller took it with + // `lockBackupDirectory` and releases it itself. The backup then does not + // take the lock, and fails once this one is lost. `quak backup` takes it // before it opens its library. - lockHeld?: boolean; + lock?: BackupLock; } export interface BackupError { @@ -502,23 +510,24 @@ const writeAlbumJSON = ( writeFileSync(path, JSON.stringify(album, null, 2)); }; -// The backup itself, which `runBackup` below runs while the lock is held. +// The backup itself, which `runBackup` below runs while `lock` is held. Once +// the lock is lost, another backup may be writing to `downloadDirectory`, so +// this one fails before it starts another file. const runLockedBackup = async ( lib: BackupLibrary, opts: BackupOptions, downloadDirectory: string, + lock: BackupLock, ): Promise => { const includeOriginals = opts.includeOriginals ?? true; const includeThumbnails = opts.includeThumbnails ?? false; const log = opts.onProgress ?? (() => {}); const only = opts.onlyAlbumNames ? new Set(opts.onlyAlbumNames) : undefined; const verify = opts.verify ?? false; - // The lock `lockBackupDirectory` took for this backup. Once it is lost, - // another backup may be writing to `downloadDirectory`. - const lock = heldLocks.get(resolve(downloadDirectory)); log("Refreshing library..."); await lib.refresh(); + if (lock.lost) throw lock.lost; const collectionsDir = join(downloadDirectory, "collections"); const thumbnailsDir = join(downloadDirectory, "thumbnails"); @@ -614,7 +623,7 @@ const runLockedBackup = async ( // that cannot be read is recorded as failed and left where it is. if (includeOriginals) { for (const [fileID, file] of distinct) { - if (lock?.lost) throw lock.lost; + if (lock.lost) throw lock.lost; let stored = storedAtSavePath(downloadDirectory, file); let mismatch = false; if (stored !== undefined && verify) { @@ -692,6 +701,7 @@ const runLockedBackup = async ( if (includeThumbnails) { for (const [fileID, file] of distinct) { + if (lock.lost) throw lock.lost; const dest = join(thumbnailsDir, `${fileID}.jpg`); if (isPresent(dest)) continue; try { @@ -723,6 +733,7 @@ const runLockedBackup = async ( log(`FAILED ML data: ${mlDataError}`); } for (const file of distinct.values()) { + if (lock.lost) throw lock.lost; const stored = storedAtSavePath(downloadDirectory, file); if (stored === undefined) continue; const path = withExtension( @@ -765,6 +776,7 @@ const runLockedBackup = async ( const albumDirNames = new Map( allCollections.map((c, i) => [c.id, dirNames[i]!]), ); + if (lock.lost) throw lock.lost; try { removeStaleAlbumDirs( collectionsDir, @@ -776,6 +788,7 @@ const runLockedBackup = async ( } for (const c of collections) { + if (lock.lost) throw lock.lost; const colDirName = albumDirNames.get(c.id)!; const colDir = join(collectionsDir, colDirName); mkdirSync(colDir, { recursive: true }); @@ -830,7 +843,7 @@ const runLockedBackup = async ( // stale entry would keep the exit code non-zero forever — a single // since-deleted photo would fail every future scheduled backup. A backup // whose lock was lost leaves the ledger as it is. - if (lock?.lost) throw lock.lost; + if (lock.lost) throw lock.lost; for (const fileID of [...ledger.keys()]) { if (!failedThisRun.has(fileID)) ledger.delete(fileID); } @@ -848,29 +861,30 @@ const runLockedBackup = async ( }; }; -// The lock this process holds on each backup directory, by the directory's -// absolute path, until it is released. `lost` is set once the lock is lost. -const heldLocks = new Map(); - // Only one backup of a directory runs at a time, in this process or another. // Creates `downloadDirectory` if it is missing, takes the lock in it and -// returns the function that releases it; while another backup holds the lock, -// fails at once with an error whose `code` is `ELOCKED`. The lock is the -// directory `backup.lock`, whose modification time proper-lockfile keeps -// current while it is held. One it has not touched for 10 seconds was left by -// a run that could not remove it, and is taken over. +// returns it; while another backup holds the lock, fails at once with an error +// whose `code` is `ELOCKED`. The lock is the directory `backup.lock`, whose +// modification time proper-lockfile keeps current while it is held. One it has +// not touched for 10 seconds was left by a run that could not remove it, and +// is taken over. // // proper-lockfile finds the lock lost when `backup.lock` is gone or is another // backup's, and calls `onCompromised` from a timer, where throwing would end -// the process. The backup holding the lock stops before it starts another file -// instead (see `runLockedBackup`), and releasing a lost lock leaves -// `backup.lock` alone. +// the process. The `onCompromised` given here sets the lock's `lost` instead, +// and the backup holding the lock stops before it starts another file (see +// `runLockedBackup`). Releasing a lost lock leaves `backup.lock` alone, since +// it may be another backup's. export const lockBackupDirectory = async ( downloadDirectory: string, -): Promise<() => Promise> => { +): Promise => { mkdirSync(downloadDirectory, { recursive: true }); - const lock: { lost?: Error } = {}; let release: () => Promise; + const lock: BackupLock = { + release: async () => { + if (lock.lost === undefined) await release(); + }, + }; try { release = await lockfile.lock(downloadDirectory, { lockfilePath: join(downloadDirectory, "backup.lock"), @@ -887,18 +901,11 @@ export const lockBackupDirectory = async ( { code: "ELOCKED" }, ); } - const path = resolve(downloadDirectory); - heldLocks.set(path, lock); - return async () => { - // Once this lock was lost, another backup in this process may have - // taken the directory. - if (heldLocks.get(path) === lock) heldLocks.delete(path); - if (lock.lost === undefined) await release(); - }; + return lock; }; // Runs the backup holding the lock, which it takes and releases itself unless -// the caller already holds it (`opts.lockHeld`). +// the caller passes the lock it took (`opts.lock`). export const runBackup = async ( lib: BackupLibrary, opts: BackupOptions, @@ -910,13 +917,13 @@ export const runBackup = async ( "open the library with one)", ); } - if (opts.lockHeld) { - return runLockedBackup(lib, opts, downloadDirectory); + if (opts.lock) { + return runLockedBackup(lib, opts, downloadDirectory, opts.lock); } - const release = await lockBackupDirectory(downloadDirectory); + const lock = await lockBackupDirectory(downloadDirectory); try { - return await runLockedBackup(lib, opts, downloadDirectory); + return await runLockedBackup(lib, opts, downloadDirectory, lock); } finally { - await release(); + await lock.release(); } }; diff --git a/src/cli-commands.ts b/src/cli-commands.ts index 154c180..254e7f7 100644 --- a/src/cli-commands.ts +++ b/src/cli-commands.ts @@ -20,7 +20,7 @@ import { type ClientSnapshot, type LoginOptions, } from "./client.js"; -import { lockBackupDirectory } from "./backup.js"; +import { lockBackupDirectory, type BackupLock } from "./backup.js"; import { init } from "./crypto/index.js"; import { defaultCacheDirectory, @@ -416,9 +416,9 @@ export const backupCommand = async ( ctx.stderr.write("Starting backup...\n"); // The lock is taken before the library opens and starts its refresh, so a // run refused while another backup of `dir` runs sends no request. - let release: () => Promise; + let lock: BackupLock; try { - release = await lockBackupDirectory(dir); + lock = await lockBackupDirectory(dir); } catch (err) { if ((err as NodeJS.ErrnoException).code !== "ELOCKED") throw err; ctx.stderr.write(`quak: ${(err as Error).message}\n`); @@ -440,7 +440,7 @@ export const backupCommand = async ( try { const result = await lib.backup({ downloadDirectory: dir, - lockHeld: true, + lock, verify: opts.verify, onProgress: (msg) => { if (!opts.json) ctx.stderr.write(msg + "\n"); @@ -475,7 +475,7 @@ export const backupCommand = async ( await lib.close(); } } finally { - await release(); + await lock.release(); } }; diff --git a/src/index.ts b/src/index.ts index 1328893..686dbef 100644 --- a/src/index.ts +++ b/src/index.ts @@ -69,6 +69,7 @@ export { type EnsureEvent, lockBackupDirectory, runBackup, + type BackupLock, type BackupOptions, type BackupResult, type BackupError, diff --git a/src/library/index.ts b/src/library/index.ts index 0c1410e..6a9f981 100644 --- a/src/library/index.ts +++ b/src/library/index.ts @@ -96,6 +96,7 @@ import { runBackup, type BackupOptions, type BackupResult } from "../backup.js"; export { lockBackupDirectory, runBackup, + type BackupLock, type BackupOptions, type BackupResult, type BackupError, @@ -570,10 +571,10 @@ export class Library { // Back up every in-scope file to `opts.downloadDirectory`, or else the // library's, each original at its save path, with a durable failure - // ledger (issue #51). Takes the lock in that directory first, unless - // `opts.lockHeld` says the caller holds it, failing at once while another - // backup of it runs, and before it starts another file once the lock is - // lost (see `lockBackupDirectory`). Waits for a + // ledger (issue #51). Takes the lock in that directory first, unless the + // caller passes the lock it took as `opts.lock`, failing at once while + // another backup of it runs, and before it starts another file once the + // lock is lost (see `lockBackupDirectory`). Waits for a // completed refresh, as `fresh()` does, joining one already running, and // rejects before touching any file but the lock when it fails. Then puts // pending originals at their save paths as `Photo.download()` does (and diff --git a/test/cli/backup-end-to-end.test.ts b/test/cli/backup-end-to-end.test.ts index c53eaa7..886a1fc 100644 --- a/test/cli/backup-end-to-end.test.ts +++ b/test/cli/backup-end-to-end.test.ts @@ -806,9 +806,10 @@ describe("a backup whose lock is lost", () => { }); // A program can take the lock before it opens its library, as `quak - // backup` does. `lockBackupDirectory` returns the lock; the program passes - // it to `lib.backup()`, which checks it as it checks a lock it took, and - // releases it once the backup ends. The lock's `lost` says why it was lost. + // backup` does: `lockBackupDirectory` returns the lock, which the program + // passes to `lib.backup()` and releases once the backup ends. The backup + // checks it as it checks a lock it took, and the lock's `lost` says why it + // was lost. it("rejects lib.backup() given its caller's lock once that lock is lost", async () => { const lock = await lockBackupDirectory(dir); const lib = await openLibrary();