Skip to content

sftp: remove the partially written temp file if writing it fails - #219

Merged
ThomasWaldmann merged 1 commit into
borgbackup:mainfrom
ThomasWaldmann:sftp-tmpfile-cleanup
Sep 6, 2026
Merged

sftp: remove the partially written temp file if writing it fails#219
ThomasWaldmann merged 1 commit into
borgbackup:mainfrom
ThomasWaldmann:sftp-tmpfile-cleanup

Conversation

@ThomasWaldmann

Copy link
Copy Markdown
Member

store() writes to a temp file and then renames it to the final name. If writing that temp file failed (e.g. the server ran out of space, or an I/O error), the partially written *.tmp file was left behind on the server: invisible to list() (TMP_SUFFIX), never removed, occupying space. Only a failing rename was cleaned up, not a failing write.

Now the temp file is removed on any failure of the write or the rename. The cleanup is best effort: if the unlink itself fails (e.g. the file was never created, or the connection is gone), the original exception is kept, which is the more interesting one and lets with_reconnect still do its job.

Tests use a fake paramiko client (like the existing test_sftp_store_memoryview_gives_bytes_to_paramiko), so they need no real server:

  • write fails (both in write and, for pipelined writes, at close time) → the temp file that was opened gets unlinked;
  • rename fails → the temp file gets unlinked (this already worked, kept as a regression guard);
  • if the cleanup unlink itself fails, the original write error still propagates.

The two write-failure tests fail on main and pass with this change.

Note: if the connection is lost mid-write, the temp file cannot be removed over the dead connection and, since store() is retried from scratch with a fresh temp name, that one temp file may be orphaned on the server. That pre-existing limitation (sftp-only, due to reconnect) is not addressed here.

🤖 Generated with Claude Code

store() writes to a temp file and then renames it to the final name. If writing
that temp file failed (e.g. server disk full, I/O error), the partially written
*.tmp file was left behind on the server: invisible to .list (TMP_SUFFIX), never
removed, occupying space. Only a failing rename was cleaned up, not a failing write.

Now the temp file is removed on any failure of the write or the rename. The
cleanup is best effort: if the unlink itself fails (e.g. the file was never
created, or the connection is gone), the original exception is kept, which is
the more interesting one and lets with_reconnect still do its job.

This mirrors the posixfs fix (borgbackup#217). Note: if the connection is lost mid-write,
the temp file cannot be removed over the dead connection and, as store() is
retried from scratch with a fresh temp name, that one temp file may be orphaned
on the server; that pre-existing limitation is not addressed here.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ThomasWaldmann
ThomasWaldmann merged commit 6a57086 into borgbackup:main Sep 6, 2026
9 checks passed
@ThomasWaldmann
ThomasWaldmann deleted the sftp-tmpfile-cleanup branch September 6, 2026 13:41
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.

1 participant