Fix Bug #3478: Fix application crash after deleting file locally (#3481)

* Fix application crash after deleting file locally by removing a TOCTTOU (Time-of-check to time-of-use) race condition when deleting a file.
* Improve inotify removal messaging when exiting application to avoid confusion
This commit is contained in:
abraunegg 2025-10-18 06:06:12 +11:00 committed by GitHub
commit c15a9e7b29
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 32 additions and 11 deletions

View file

@ -1061,6 +1061,9 @@ int main(string[] cliArgs) {
} else {
addLogEntry("Cannot delete remote item: " ~ e.msg, ["info", "notify"]);
}
} catch (FileException e) {
// Path is gone locally, log and continue.
addLogEntry("ERROR: The local file system returned an error with the following message: " ~ e.msg, ["verbose"]);
} catch (Exception e) {
addLogEntry("Cannot delete remote item: " ~ e.msg, ["info", "notify"]);
}

View file

@ -511,7 +511,7 @@ final class Monitor {
synchronized(inotifyMutex) {
int ret = worker.removeInotifyWatch(wd);
if (ret < 0) throw new MonitorException("inotify_rm_watch failed");
if (verboseLogging) {addLogEntry("Monitored directory removed: " ~ to!string(wdToDirName[wd]), ["verbose"]);}
if (verboseLogging) {addLogEntry("Stopped monitoring directory (inotify watch removed): " ~ to!string(wdToDirName[wd]), ["verbose"]);}
wdToDirName.remove(wd);
}
}
@ -524,7 +524,7 @@ final class Monitor {
int ret = worker.removeInotifyWatch(wd);
if (ret < 0) throw new MonitorException("inotify_rm_watch failed");
wdToDirName.remove(wd);
if (verboseLogging) {addLogEntry("Monitored directory removed: " ~ dirname, ["verbose"]);}
if (verboseLogging) {addLogEntry("Stopped monitoring directory (inotify watch removed): " ~ dirname, ["verbose"]);}
}
}
}

View file

@ -4582,13 +4582,13 @@ class SyncEngine {
// Perform the action
if (!dryRun) {
if (isFile(path)) {
remove(path);
safeRemove(path);
} else {
try {
// Remove any children of this path if they still exist
// Resolve 'Directory not empty' error when deleting local files
foreach (DirEntry child; dirEntries(path, SpanMode.depth, false)) {
attrIsDir(child.linkAttributes) ? rmdir(child.name) : remove(child.name);
attrIsDir(child.linkAttributes) ? rmdir(child.name) : safeRemove(child.name);
}
// Remove the path now that it is empty of children
rmdirRecurse(path);
@ -7629,7 +7629,7 @@ class SyncEngine {
// No --dry-run ... process local delete
if (exists(child)) {
try {
attrIsDir(child.linkAttributes) ? rmdir(child.name) : remove(child.name);
attrIsDir(child.linkAttributes) ? rmdir(child.name) : safeRemove(child.name);
} catch (FileException e) {
// display the error message
displayFileSystemErrorMessage(e.msg, thisFunctionName);
@ -9561,7 +9561,7 @@ class SyncEngine {
if (verboseLogging) {addLogEntry("File upload session failed - invalid calculation of fragment size", ["verbose"]);}
if (exists(threadUploadSessionFilePath)) {
remove(threadUploadSessionFilePath);
safeRemove(threadUploadSessionFilePath);
}
// set uploadResponse to null as error
uploadResponse = null;
@ -9736,7 +9736,7 @@ class SyncEngine {
// cleanup session data
if (exists(threadUploadSessionFilePath)) {
remove(threadUploadSessionFilePath);
safeRemove(threadUploadSessionFilePath);
}
// set uploadResponse to null as error
uploadResponse = null;
@ -9753,7 +9753,7 @@ class SyncEngine {
// Remove session file if it exists
if (exists(threadUploadSessionFilePath)) {
remove(threadUploadSessionFilePath);
safeRemove(threadUploadSessionFilePath);
}
// Display function processing time if configured to do so
@ -11423,6 +11423,9 @@ class SyncEngine {
} else {
uploadDeletedItem(dbItem, path);
}
} catch (FileException e) {
// filesystem generated an error message - display error message
displayFileSystemErrorMessage(e.msg, thisFunctionName);
} catch (OneDriveException e) {
if (e.httpStatusCode == 404) {
addLogEntry(e.msg);
@ -12733,7 +12736,7 @@ class SyncEngine {
// cleanup session path
if (exists(sessionFilePath)) {
if (!dryRun) {
remove(sessionFilePath);
safeRemove(sessionFilePath);
}
}
}
@ -12787,7 +12790,7 @@ class SyncEngine {
// Cleanup 'resume_download' file
if (exists(resumeDownloadFile)) {
if (!dryRun) {
remove(resumeDownloadFile);
safeRemove(resumeDownloadFile);
}
}
}

View file

@ -3,6 +3,7 @@ module util;
// What does this module require to function?
import core.stdc.stdlib: EXIT_SUCCESS, EXIT_FAILURE, exit;
import core.stdc.errno : ENOENT;
import std.base64;
import std.conv;
import std.digest.crc;
@ -172,7 +173,21 @@ void safeRename(const(char)[] oldPath, const(char)[] newPath, bool dryRun) {
// Deletes the specified file without throwing an exception if it does not exists
void safeRemove(const(char)[] path) {
if (exists(path)) remove(path);
// Set this function name
string thisFunctionName = format("%s.%s", strip(__MODULE__) , strip(getFunctionName!({})));
// Attempt the local deletion
try {
// Attempt once; no pre-check to avoid TOCTTOU
remove(path); // attempt once, no pre-check
return; // removed
} catch (FileException e) {
if (e.errno == ENOENT) { // already gone → fine
return; // nothing to do
}
// Anything else is noteworthy (EISDIR, EACCES, etc.)
displayFileSystemErrorMessage(e.msg, thisFunctionName);
}
}
// Returns the SHA1 hash hex string of a file, or an empty string on failure