fix(downloads): commit archive reservation and ownership atomically

This commit is contained in:
4gray committed 2026-09-08 05:35:36 +02:00
1 parent 4f55bc5e66
commit e257b77fca
8 files changed
+154 -21

No files matched your search

+2 -1
View File
@@ -1045,7 +1045,8 @@ so Retry can reserve a fresh path. A synchronous completion-commit boundary
rejects late pause/cancel commands before awaited cleanup and persistence.
Archive ownership includes device, inode and positive creation time to reject
reused inodes after unlink; old proofs without creation time remain untrusted.
Fresh reservations capture and journal ownership before the initial HTTP wait;
Fresh reservations atomically commit their row path/name and captured ownership
before the initial HTTP wait;
no preexisting partial is truncated without matching expected ownership.
Private cleanup captures are journaled before relocation, keeping failed
Remove/Clear/cancel cleanup retryable across restarts without hardlinks. Active
+2 -1
View File
@@ -1856,7 +1856,8 @@ so Retry can reserve a fresh path. A synchronous completion-commit boundary
rejects late pause/cancel commands before awaited cleanup and persistence.
Archive ownership includes device, inode and positive creation time to reject
reused inodes after unlink; old proofs without creation time remain untrusted.
Fresh reservations capture and journal ownership before the initial HTTP wait;
Fresh reservations atomically commit their row path/name and captured ownership
before the initial HTTP wait;
no preexisting partial is truncated without matching expected ownership.
Private cleanup captures are journaled before relocation, keeping failed
Remove/Clear/cancel cleanup retryable across restarts without hardlinks. Active
@@ -0,0 +1,88 @@
import { execFileSync } from 'node:child_process';
import { createRequire } from 'node:module';
import { resolve } from 'node:path';
import { pathToFileURL } from 'node:url';
function runReservationScenario(scenario: string): {
row: { file_path: string | null; file_name: string };
proofs: { download_id: number; proof: string }[];
error?: string;
} {
const journalUrl = pathToFileURL(
resolve(__dirname, 'download-catchup-journal.ts')
).href;
const script = `
const { default: Database } = await import('better-sqlite3');
const { drizzle } = await import('drizzle-orm/better-sqlite3');
const { recordArchiveReservation } = await import(${JSON.stringify(journalUrl)});
const sqlite = new Database(':memory:');
sqlite.exec(\`
CREATE TABLE downloads (id INTEGER PRIMARY KEY, file_path TEXT, file_name TEXT, updated_at TEXT);
CREATE TABLE download_archive_finalizations (download_id INTEGER PRIMARY KEY REFERENCES downloads(id), proof TEXT NOT NULL);
INSERT INTO downloads (id, file_name) VALUES (1, 'show.ts');
\`);
const db = drizzle(sqlite);
const identity = { dev: 1, ino: 2, birthtimeMs: 1000 };
let error;
${scenario}
console.log(JSON.stringify({ error,
row: sqlite.prepare('SELECT file_path, file_name FROM downloads WHERE id=1').get(),
proofs: sqlite.prepare('SELECT * FROM download_archive_finalizations').all()
}));
sqlite.close();
`;
return JSON.parse(
execFileSync(
createRequire(__filename)('electron'),
['--import', 'tsx', '--eval', script],
{
cwd: process.cwd(),
encoding: 'utf8',
env: {
...process.env,
ELECTRON_RUN_AS_NODE: '1',
TSX_TSCONFIG_PATH: resolve(
process.cwd(),
'tsconfig.base.json'
),
},
}
)
);
}
it('commits the reservation path and ownership together before transfer setup', () => {
const result = runReservationScenario(`
await recordArchiveReservation(db, 1, '/archive/show (1).ts', identity, 'show (1).ts');
`);
expect(result.row).toEqual({
file_path: '/archive/show (1).ts',
file_name: 'show (1).ts',
});
expect(JSON.parse(result.proofs[0].proof)).toMatchObject({
phase: 'transfer',
filePath: result.row.file_path,
partialIdentity: { dev: 1, ino: 2, birthtimeMs: 1000 },
});
});
it('rolls back the row path when the ownership write fails', () => {
const result = runReservationScenario(`
sqlite.exec("CREATE TRIGGER reject_proof BEFORE INSERT ON download_archive_finalizations BEGIN SELECT RAISE(ABORT, 'journal unavailable'); END");
try { await recordArchiveReservation(db, 1, '/archive/new.ts', identity, 'new.ts'); }
catch (failure) { error = failure.message; }
`);
expect(result.error).toContain('journal unavailable');
expect(result.row).toEqual({ file_path: null, file_name: 'show.ts' });
expect(result.proofs).toEqual([]);
});
it('rejects a reservation whose download row disappeared', () => {
const result = runReservationScenario(`
try { await recordArchiveReservation(db, 2, '/archive/new.ts', identity, 'new.ts'); }
catch (failure) { error = failure.message; }
`);
expect(result.error).toContain('row is unavailable');
expect(result.proofs).toEqual([]);
expect(result.row.file_path).toBeNull();
});
@@ -1,5 +1,5 @@
import { cleanupArchiveCapture } from './download-catchup-capture';
import { eq, inArray } from 'drizzle-orm';
import { eq, inArray, sql } from 'drizzle-orm';
import { lstatSync } from 'node:fs';
import { isAbsolute } from 'node:path';
import * as schema from '../../database/schema';
@@ -46,6 +46,35 @@ export async function recordArchivePartial(
});
}
/** The row path and its ownership proof must become recoverable together. */
export async function recordArchiveReservation(
db: DownloadsDatabase,
downloadId: number,
filePath: string,
partialIdentity: ArchiveFileIdentity,
fileName: string
): Promise<void> {
db.transaction((tx) => {
const result = tx
.update(schema.downloads)
.set({
filePath,
fileName,
updatedAt: sql`CURRENT_TIMESTAMP`,
})
.where(eq(schema.downloads.id, downloadId))
.run();
if (result.changes !== 1)
throw new Error('Archive reservation row is unavailable');
writeArchiveProof(tx, downloadId, {
version: 1,
phase: 'transfer',
filePath,
partialIdentity,
}).run();
});
}
export async function recordArchiveFinalization(
db: DownloadsDatabase,
downloadId: number,
@@ -54,11 +83,11 @@ export async function recordArchiveFinalization(
await writeArchiveProof(db, downloadId, proof);
}
async function writeArchiveProof(
db: DownloadsDatabase,
function writeArchiveProof(
db: Pick<DownloadsDatabase, 'insert'>,
downloadId: number,
proof: ArchiveDownloadProof
): Promise<void> {
) {
const serialized = JSON.stringify({
...proof,
partialIdentity: archiveFileIdentity(proof.partialIdentity),
@@ -68,7 +97,7 @@ async function writeArchiveProof(
}
: {}),
});
await db
return db
.insert(schema.downloadArchiveFinalizations)
.values({ downloadId, proof: serialized })
.onConflictDoUpdate({
@@ -13,7 +13,7 @@ import { reserveTarget } from './download-runtime-reservation';
import { reserveFreshCatchupTarget } from './download-catchup-reservation';
import {
clearArchiveFinalization,
recordArchivePartial,
recordArchiveReservation,
readArchiveFinalizations,
} from './download-catchup-journal';
import type { DownloadsDatabase, DownloadTask } from './download-task';
@@ -23,7 +23,7 @@ jest.mock('./download-catchup-journal', () => ({
clearArchiveFinalization: jest.fn().mockResolvedValue(undefined),
readArchiveFinalizations: jest.fn(),
recordArchiveCleanupPath: jest.fn(),
recordArchivePartial: jest.fn().mockResolvedValue(undefined),
recordArchiveReservation: jest.fn().mockResolvedValue(undefined),
}));
it.each([false, true])(
@@ -156,11 +156,12 @@ it('binds a fresh reservation before a replacement can arrive during the HTTP wa
};
try {
const reservation = await reserveTarget({} as DownloadsDatabase, task);
expect(recordArchivePartial).toHaveBeenCalledWith(
expect(recordArchiveReservation).toHaveBeenCalledWith(
expect.anything(),
task.id,
reservation.path,
task.catchupExpectedPartialIdentity
task.catchupExpectedPartialIdentity,
reservation.filename
);
await rename(reservation.partialPath, join(directory, 'original'));
await writeFile(
@@ -200,13 +201,18 @@ it.each([false, true])(
},
};
const failure = new Error('SQLite write failed');
jest.mocked(recordArchivePartial).mockImplementationOnce(async () => {
if (replaced) {
await rename(filePath + '.part', join(directory, 'original'));
await writeFile(filePath + '.part', 'foreign bytes');
jest.mocked(recordArchiveReservation).mockImplementationOnce(
async () => {
if (replaced) {
await rename(
filePath + '.part',
join(directory, 'original')
);
await writeFile(filePath + '.part', 'foreign bytes');
}
throw failure;
}
throw failure;
});
);
try {
await expect(
reserveTarget({} as DownloadsDatabase, task)
@@ -4,7 +4,7 @@ import { cleanupCatchupPartial } from './download-catchup-cleanup';
import { cleanupStoredCatchupPartial } from './download-catchup-removal';
import {
clearArchiveFinalization,
recordArchivePartial,
recordArchiveReservation,
} from './download-catchup-journal';
import {
ArchivePartialReplacedError,
@@ -69,7 +69,13 @@ export async function reserveOwnedCatchupTarget(
if (!identity)
throw new Error('Archive reservation identity is unavailable');
try {
await recordArchivePartial(db, task.id, reservation.path, identity);
await recordArchiveReservation(
db,
task.id,
reservation.path,
identity,
reservation.filename
);
} catch (error) {
// No media bytes have been written. Without a usable journal, remove
// only the empty reservation whose identity came from our descriptor.
@@ -49,7 +49,7 @@ function mockRememberCapture(
}
jest.mock('./download-catchup-journal', () => ({
...jest.requireActual('./download-catchup-journal'),
recordArchivePartial: jest.fn(
recordArchiveReservation: jest.fn(
async (_db, id, filePath, partialIdentity) => {
mockProofs.set(id, {
version: 1,
+3 -1
View File
@@ -104,7 +104,9 @@ unknown or replaced entries are preserved. Ownership includes a positive file
creation timestamp as well as device/inode, so inode reuse after unlink cannot
bless a new entry; proofs lacking creation time remain untrusted. Fresh
reservations capture this identity from their exclusive creation descriptor and
journal it before the HTTP wait. Existing partials without matching expected
commit it together with the downloads row path/name in one SQLite transaction
before the HTTP wait. A committed reservation is therefore recoverable even if
the process exits before transfer setup. Existing partials without matching expected
ownership are never truncated, including before the first response. Before capture, a synchronous SQLite
write records `partialCleanupPath` (or `finalCleanupPath` for failed promotion)
in the existing ownership proof. Active cancellation/failure, promotion and