Update safeRemove() to retry on EINTR / EBUSY filesystem responses (#3589)

This change updates safeBackup() to use the hardened safeRename() implementation when renaming local files to preserve existing data.

**What changed**
- safeBackup() now delegates file renaming to safeRename() instead of calling std.file.rename() directly.
- The result of the rename operation is checked via the boolean return value from safeRename() before updating renamedPath.
- Transient filesystem errors (EINTR, EBUSY) are now handled consistently during backup renames.
- Cross-filesystem rename failures (EXDEV) and other non-retryable errors are logged and handled safely.

**Why this is needed**
The previous implementation performed a direct rename() inside a local try/catch, which:
- Did not retry on interruptible system calls (EINTR)
- Could incorrectly treat transient filesystem conditions as hard failures
- Did not share the same resilience and logging behaviour as other filesystem operations

This change ensures that backup renames benefit from the same bounded retry logic and error handling already applied to safeRemove() and safeRename().

**Scope and behaviour**
- No change to functional behaviour for successful backups.
- Dry-run behaviour is unchanged.
- Genuine filesystem errors are still logged and surfaced.
- Reduces noisy or misleading error logs caused by transient rename failures, particularly during shutdown or signal handling.

This brings safeBackup() into alignment with the updated filesystem-safety model used elsewhere in the codebase and improves reliability when preserving local data.
This commit is contained in:
abraunegg 2025-12-31 14:11:05 +11:00 committed by GitHub
commit 324c837ff6
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -168,27 +168,14 @@ void safeBackup(const(char)[] path, bool dryRun, bool bypassDataPreservation, ou
// Perform (or simulate) the rename
if (!dryRun) {
// Not a --dry-run scenario - attempt the file rename
//
// There are 2 options to rename a file
// rename() - https://dlang.org/library/std/file/rename.html
// std.file.copy() - https://dlang.org/library/std/file/copy.html
//
// rename:
// It is not possible to rename a file across different mount points or drives. On POSIX, the operation is atomic. That means, if to already exists there will be no time period during the operation where to is missing.
//
// std.file.copy
// Copy file from to file to. File timestamps are preserved. File attributes are preserved, if preserve equals Yes.preserveAttributes
//
// Use rename() as Linux is POSIX compliant, we have an atomic operation where at no point in time the 'to' is missing.
try {
rename(spath, candidate); // POSIX atomic on same mount
renamedPath = candidate;
} catch (Exception e) {
addLogEntry("Renaming of local file failed for " ~ spath ~ ": " ~ e.msg, ["error"]);
}
// Not a --dry-run scenario - attempt the file rename to create a safe backup
// Use safeRename()
if (safeRename(spath, candidate, dryRun)) {
renamedPath = candidate;
} else {
// Failed to rename using safeRename()
addLogEntry("Renaming of local file failed for " ~ spath ~ " -> " ~ candidate, ["error"]);
}
} else {
if (debugLogging) {
addLogEntry("DRY-RUN: Skipping renaming local file to preserve existing file and prevent data loss: " ~ spath ~ " -> " ~ candidate, ["debug"]);
@ -197,12 +184,12 @@ void safeBackup(const(char)[] path, bool dryRun, bool bypassDataPreservation, ou
}
// Rename the given item, and only performs the function if not in a --dry-run scenario
void safeRename(const(char)[] oldPath, const(char)[] newPath, bool dryRun) {
bool safeRename(const(char)[] oldPath, const(char)[] newPath, bool dryRun) {
string thisFunctionName = format("%s.%s", strip(__MODULE__), strip(getFunctionName!({})));
if (dryRun) {
if (debugLogging) { addLogEntry("DRY-RUN: Skipping local file rename", ["debug"]); }
return;
return true;
}
int maxAttempts = 5;
@ -210,8 +197,21 @@ void safeRename(const(char)[] oldPath, const(char)[] newPath, bool dryRun) {
foreach (attempt; 0 .. maxAttempts) {
try {
if (debugLogging) { addLogEntry("Calling rename(oldPath, newPath)", ["debug"]); }
// There are 2 options to rename a file
// rename() - https://dlang.org/library/std/file/rename.html
// std.file.copy() - https://dlang.org/library/std/file/copy.html
//
// rename:
// It is not possible to rename a file across different mount points or drives. On POSIX, the operation is atomic. That means, if to already exists there will be no time period during the operation where to is missing.
//
// std.file.copy
// Copy file from to file to. File timestamps are preserved. File attributes are preserved, if preserve equals Yes.preserveAttributes
//
// Use rename() as Linux is POSIX compliant, we have an atomic operation where at no point in time the 'to' is missing.
rename(oldPath, newPath);
return;
return true;
} catch (FileException e) {
// Retry on EINTR
if (e.errno == EINTR) { // Interrupted by signal → retry
@ -228,17 +228,18 @@ void safeRename(const(char)[] oldPath, const(char)[] newPath, bool dryRun) {
// Cross-device rename: not retryable
if (e.errno == EXDEV) {
displayFileSystemErrorMessage("Rename failed (cross-filesystem): " ~ e.msg, thisFunctionName, "oldPath=" ~ to!string(oldPath) ~ " newPath=" ~ to!string(newPath));
return;
return false;
}
// Everything else: log once and return
displayFileSystemErrorMessage(e.msg, thisFunctionName, "oldPath=" ~ to!string(oldPath) ~ " newPath=" ~ to!string(newPath));
return;
return false;
}
}
// If we get here, we exhausted retries
// Log the last failure
displayFileSystemErrorMessage("Failed to rename after retries: ", thisFunctionName, "oldPath=" ~ to!string(oldPath) ~ " newPath=" ~ to!string(newPath));
return false;
}
// Deletes the specified file without throwing an exception if there is an issue