From c15a9e7b29f03d3ce50e65a75c6c0c78c6bdb1e1 Mon Sep 17 00:00:00 2001 From: abraunegg Date: Sat, 18 Oct 2025 06:06:12 +1100 Subject: [PATCH] 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 --- src/main.d | 3 +++ src/monitor.d | 4 ++-- src/sync.d | 19 +++++++++++-------- src/util.d | 17 ++++++++++++++++- 4 files changed, 32 insertions(+), 11 deletions(-) diff --git a/src/main.d b/src/main.d index 3e1876f2..b0c95cfa 100644 --- a/src/main.d +++ b/src/main.d @@ -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"]); } diff --git a/src/monitor.d b/src/monitor.d index 99100f63..d10f8840 100644 --- a/src/monitor.d +++ b/src/monitor.d @@ -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"]);} } } } diff --git a/src/sync.d b/src/sync.d index f09b059a..e840e706 100644 --- a/src/sync.d +++ b/src/sync.d @@ -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); } } } diff --git a/src/util.d b/src/util.d index d968f289..ee2f390d 100644 --- a/src/util.d +++ b/src/util.d @@ -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