Skip to content

A stale .bak silently disables the re-fetch backup on Windows - #776

Merged
xroche merged 6 commits into
masterfrom
fix/backup-stale-bak
Jul 27, 2026
Merged

A stale .bak silently disables the re-fetch backup on Windows#776
xroche merged 6 commits into
masterfrom
fix/backup-stale-bak

Conversation

@xroche

@xroche xroche commented Jul 27, 2026

Copy link
Copy Markdown
Owner

back_finalize() moves an existing file aside to <file>.bak before truncating it, so an aborted re-fetch can put the previous copy back. That rename was a bare RENAME, and Windows refuses to rename onto an existing target, so a .bak outliving a killed run made every later re-fetch of that URL fail the rename, take the tmpfile = NULL branch with nothing logged, and truncate the live file with no backup at all. The #77 guard was off for that file, silently and for good. hts_rename_over() (#754) unlinks and retries, which fixes it.

Worth a look, because clobbering a stale .bak is not free. A run killed between the rename-aside and the finalize leaves the previous complete copy in .bak and a partial in the live file, and the next re-fetch now overwrites the good one. It still looks like the right trade: POSIX rename() has behaved exactly this way since #77 landed, so the alternative is leaving Windows with a guard that stays dead until someone deletes the file by hand, and a leftover .bak is engine garbage that nothing advertises or ever restores. The clobber is logged now, so it is at least visible.

Any failure to create the backup is logged too, instead of quietly disabling the safety net. htscache.c's static hts_rename() wrapper and the hand-rolled old.zip unlink at its only call site go the same way, which removes a copy of the unlink-then-rename idiom rather than adding a fifth.

Test 101 plants both kinds of leftover before the update pass: a stale file, which has to be clobbered so the -M abort can still restore the pass-1 copy, and a directory, which cannot be clobbered and has to be reported instead. Only the Windows leg arms the first half, since POSIX clobbers on its own; the second is armed everywhere. Test 37 gains a live update pass so the dead pass rotates onto an existing old.zip, which nothing covered.

Two older bugs on the same path came out of the review and are filed separately: #774 (the .bak name collides with a mirrored file of the same name, reproduced) and #775 (a failed filecreate() on a chunked re-fetch commits the backup away).

Closes #758

xroche and others added 5 commits July 27, 2026 09:44
back_finalize() moved an existing file aside with a bare RENAME before
truncating it. Windows refuses to rename onto an existing target, so a
.bak outliving a killed run made every later re-fetch of that URL take
the tmpfile = NULL branch: no backup, and nothing to restore when the
transfer aborted. Use hts_rename_over(), which clobbers, and log when
the backup cannot be made at all.

htscache.c's hts_rename() wrapper and its caller's hand-rolled old.zip
unlink go the same way.

Closes #758

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>
Report when a leftover .bak is about to be replaced, so destroying the
orphan of a killed run is not silent. Test 96 now asserts the -M abort
actually fired and that fast.bin stays a file; test 37 gains a live
update pass so the dead pass rotates onto an existing old.zip, which
nothing covered.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>
96 was taken by #763 on master.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>
master took 97 and 98 for the WARC tests.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>
xroche added a commit that referenced this pull request Jul 27, 2026
master took 97 and 98 for the WARC tests, and 99 is claimed by #776.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>
99 and 100 went to #783 and #780.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>
@xroche
xroche merged commit 4fd6767 into master Jul 27, 2026
22 checks passed
xroche added a commit that referenced this pull request Jul 27, 2026
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>
xroche added a commit that referenced this pull request Jul 27, 2026
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>
@xroche
xroche deleted the fix/backup-stale-bak branch July 27, 2026 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A leftover .bak silently turns off the #77 re-fetch backup on Windows

1 participant