fix(downloads): require durable ownership before cleanup relocation

This commit is contained in:
4gray committed 2026-09-08 06:00:46 +02:00
1 parent b05db7ca21
commit 45d5371250
7 files changed
+113 -21

No files matched your search

@@ -78,7 +78,9 @@ it.each(['EEXIST', 'ENOTSUP'])(
.spyOn(console, 'warn')
.mockImplementation(() => undefined);
try {
await cleanupCatchupFile(path, identity);
await expect(cleanupCatchupFile(path, identity)).rejects.toThrow(
'cannot restore'
);
const quarantine = (await readdir(directory)).find((entry) =>
entry.startsWith('.iptvnator-cleanup-')
);
@@ -25,11 +25,12 @@ export async function cleanupCatchupFile(
try {
await link(captured, path);
await unlink(captured);
} catch {
} catch (error) {
console.warn(
'[Downloads] Replaced file retained for recovery:',
captured
);
throw error;
}
}
} finally {
@@ -337,3 +337,26 @@ it('never truncates a preexisting file without expected ownership', async () =>
expect(record).not.toHaveBeenCalled();
expect(readFileSync(filePath + '.part', 'utf8')).toBe('owned bytes');
});
it.each([false, true])(
'does not relocate an unproved fallback target (replaced=%s)',
async (replaced) => {
writeFileSync(filePath, '');
const createdIdentity = lstatSync(filePath);
if (replaced) {
renameSync(filePath, join(directory, 'empty-original'));
writeFileSync(filePath, 'foreign final');
}
const db = {
select: () => ({ from: () => ({ where: async () => [] }) }),
};
jest.mocked(renameSync).mockClear();
await expect(
cleanupStoredCatchupFinal(db as never, 1, filePath, createdIdentity)
).resolves.toBe(false);
expect(readFileSync(filePath, 'utf8')).toBe(
replaced ? 'foreign final' : ''
);
expect(renameSync).not.toHaveBeenCalled();
}
);
@@ -1,4 +1,3 @@
import { cleanupCatchupFile } from './download-catchup-cleanup';
import { cleanupArchiveCapture } from './download-catchup-capture';
import {
lstatSync,
@@ -133,11 +132,9 @@ export async function cleanupStoredCatchupFinal(
(createdIdentity &&
!sameArchiveFileIdentity(createdIdentity, proof.finalIdentity))
) {
// Only the exclusively created empty target can precede final proof;
// no copy bytes are written until its identity has committed.
if (createdIdentity)
await cleanupCatchupFile(filePath, createdIdentity);
return true;
// No copy bytes precede final proof. Preserve an unproved empty
// destination in place: relocation needs a durable capture pointer.
return !createdIdentity;
}
cleanupArchiveCapture(proof);
removeOwnedEntry(filePath, proof.finalIdentity, (path) =>
@@ -201,8 +201,8 @@ it.each([false, true])(
},
};
const failure = new Error('SQLite write failed');
jest.mocked(recordArchiveReservation).mockImplementationOnce(
async () => {
jest.mocked(recordArchiveReservation)
.mockImplementationOnce(async () => {
if (replaced) {
await rename(
filePath + '.part',
@@ -211,8 +211,22 @@ it.each([false, true])(
await writeFile(filePath + '.part', 'foreign bytes');
}
throw failure;
}
);
})
.mockImplementationOnce(async (_db, id, path, identity) => {
jest.mocked(readArchiveFinalizations).mockResolvedValue(
new Map([
[
id,
{
version: 1,
phase: 'transfer',
filePath: path,
partialIdentity: identity,
},
],
])
);
});
try {
await expect(
reserveTarget({} as DownloadsDatabase, task)
@@ -231,3 +245,45 @@ it.each([false, true])(
}
}
);
it.each([false, true])(
'never relocates an unjournaled reservation when SQLite remains unavailable (replaced=%s)',
async (replaced) => {
const directory = await mkdtemp(join(tmpdir(), 'archive-no-journal-'));
const filePath = join(directory, 'show.ts');
const task: DownloadTask = {
id: 21,
directory,
fileName: 'show.ts',
url: 'https://provider.test/archive.ts',
catchup: {
channelName: 'News',
startTimestamp: 100,
stopTimestamp: 200,
},
};
const failure = new Error('SQLite unavailable');
jest.mocked(recordArchiveReservation)
.mockImplementationOnce(async () => {
if (replaced) {
await rename(
filePath + '.part',
join(directory, 'original')
);
await writeFile(filePath + '.part', 'foreign bytes');
}
throw failure;
})
.mockRejectedValueOnce(failure);
try {
await expect(
reserveTarget({} as DownloadsDatabase, task)
).rejects.toBe(failure);
expect(await readFile(filePath + '.part', 'utf8')).toBe(
replaced ? 'foreign bytes' : ''
);
} finally {
await rm(directory, { recursive: true, force: true });
}
}
);
@@ -1,6 +1,5 @@
import { closeSync, fstatSync, openSync } from 'node:fs';
import { lstat } from 'node:fs/promises';
import { cleanupCatchupPartial } from './download-catchup-cleanup';
import { cleanupStoredCatchupPartial } from './download-catchup-removal';
import {
clearArchiveFinalization,
@@ -77,9 +76,21 @@ export async function reserveOwnedCatchupTarget(
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.
await cleanupCatchupPartial(reservation.path, identity);
// Retry persistence once before cleanup. Never relocate an unjournaled
// entry: if SQLite remains unavailable, leave the empty placeholder in
// place rather than risk losing the recovery path of a replacement.
try {
await recordArchiveReservation(
db,
task.id,
reservation.path,
identity,
reservation.filename
);
await cleanupStoredCatchupPartial(db, task.id, reservation.path);
} catch {
/* no media bytes exist before the first ownership commit */
}
throw error;
}
return reservation;
+7 -5
View File
@@ -123,11 +123,13 @@ filesystems without hardlinks. Cleanup remains synchronous after the
runtime guard, so a completion transition cannot interleave with unlink.
Missing archive re-downloads claim a fresh reservation instead of inheriting the
completed file's identity.
If the initial reservation journal write fails, descriptor-owned cleanup removes
the empty partial while preserving a replacement. A kill between exclusive
reservation/copy-file creation and its identity journal commit (or a failure of
that unjournaled cleanup) can leave an unowned **empty** destination: no bytes
are written before the commit.
If the initial reservation journal write fails, persistence is retried once
before journal-backed cleanup. If SQLite remains unavailable, the empty
reservation stays at its public path; no unjournaled entry is relocated. An
empty fallback-copy target whose final proof cannot commit is likewise preserved.
A kill between exclusive reservation/copy-file creation and its identity journal
commit, or persistent SQLite failure before that commit, can leave an unowned
**empty** destination: no bytes are written before the commit.
Recovery preserves that file rather than guessing ownership; Retry uses a
numbered free destination. SQLite and filesystem creation cannot commit
atomically, and portable rename cannot guarantee no-clobber publication on the