Skip to content

Commit c15a9e7

Browse files
authored
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
1 parent 46ec991 commit c15a9e7

4 files changed

Lines changed: 32 additions & 11 deletions

File tree

src/main.d

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1061,6 +1061,9 @@ int main(string[] cliArgs) {
10611061
} else {
10621062
addLogEntry("Cannot delete remote item: " ~ e.msg, ["info", "notify"]);
10631063
}
1064+
} catch (FileException e) {
1065+
// Path is gone locally, log and continue.
1066+
addLogEntry("ERROR: The local file system returned an error with the following message: " ~ e.msg, ["verbose"]);
10641067
} catch (Exception e) {
10651068
addLogEntry("Cannot delete remote item: " ~ e.msg, ["info", "notify"]);
10661069
}

src/monitor.d

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -511,7 +511,7 @@ final class Monitor {
511511
synchronized(inotifyMutex) {
512512
int ret = worker.removeInotifyWatch(wd);
513513
if (ret < 0) throw new MonitorException("inotify_rm_watch failed");
514-
if (verboseLogging) {addLogEntry("Monitored directory removed: " ~ to!string(wdToDirName[wd]), ["verbose"]);}
514+
if (verboseLogging) {addLogEntry("Stopped monitoring directory (inotify watch removed): " ~ to!string(wdToDirName[wd]), ["verbose"]);}
515515
wdToDirName.remove(wd);
516516
}
517517
}
@@ -524,7 +524,7 @@ final class Monitor {
524524
int ret = worker.removeInotifyWatch(wd);
525525
if (ret < 0) throw new MonitorException("inotify_rm_watch failed");
526526
wdToDirName.remove(wd);
527-
if (verboseLogging) {addLogEntry("Monitored directory removed: " ~ dirname, ["verbose"]);}
527+
if (verboseLogging) {addLogEntry("Stopped monitoring directory (inotify watch removed): " ~ dirname, ["verbose"]);}
528528
}
529529
}
530530
}

src/sync.d

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4582,13 +4582,13 @@ class SyncEngine {
45824582
// Perform the action
45834583
if (!dryRun) {
45844584
if (isFile(path)) {
4585-
remove(path);
4585+
safeRemove(path);
45864586
} else {
45874587
try {
45884588
// Remove any children of this path if they still exist
45894589
// Resolve 'Directory not empty' error when deleting local files
45904590
foreach (DirEntry child; dirEntries(path, SpanMode.depth, false)) {
4591-
attrIsDir(child.linkAttributes) ? rmdir(child.name) : remove(child.name);
4591+
attrIsDir(child.linkAttributes) ? rmdir(child.name) : safeRemove(child.name);
45924592
}
45934593
// Remove the path now that it is empty of children
45944594
rmdirRecurse(path);
@@ -7629,7 +7629,7 @@ class SyncEngine {
76297629
// No --dry-run ... process local delete
76307630
if (exists(child)) {
76317631
try {
7632-
attrIsDir(child.linkAttributes) ? rmdir(child.name) : remove(child.name);
7632+
attrIsDir(child.linkAttributes) ? rmdir(child.name) : safeRemove(child.name);
76337633
} catch (FileException e) {
76347634
// display the error message
76357635
displayFileSystemErrorMessage(e.msg, thisFunctionName);
@@ -9561,7 +9561,7 @@ class SyncEngine {
95619561
if (verboseLogging) {addLogEntry("File upload session failed - invalid calculation of fragment size", ["verbose"]);}
95629562

95639563
if (exists(threadUploadSessionFilePath)) {
9564-
remove(threadUploadSessionFilePath);
9564+
safeRemove(threadUploadSessionFilePath);
95659565
}
95669566
// set uploadResponse to null as error
95679567
uploadResponse = null;
@@ -9736,7 +9736,7 @@ class SyncEngine {
97369736

97379737
// cleanup session data
97389738
if (exists(threadUploadSessionFilePath)) {
9739-
remove(threadUploadSessionFilePath);
9739+
safeRemove(threadUploadSessionFilePath);
97409740
}
97419741
// set uploadResponse to null as error
97429742
uploadResponse = null;
@@ -9753,7 +9753,7 @@ class SyncEngine {
97539753

97549754
// Remove session file if it exists
97559755
if (exists(threadUploadSessionFilePath)) {
9756-
remove(threadUploadSessionFilePath);
9756+
safeRemove(threadUploadSessionFilePath);
97579757
}
97589758

97599759
// Display function processing time if configured to do so
@@ -11423,6 +11423,9 @@ class SyncEngine {
1142311423
} else {
1142411424
uploadDeletedItem(dbItem, path);
1142511425
}
11426+
} catch (FileException e) {
11427+
// filesystem generated an error message - display error message
11428+
displayFileSystemErrorMessage(e.msg, thisFunctionName);
1142611429
} catch (OneDriveException e) {
1142711430
if (e.httpStatusCode == 404) {
1142811431
addLogEntry(e.msg);
@@ -12733,7 +12736,7 @@ class SyncEngine {
1273312736
// cleanup session path
1273412737
if (exists(sessionFilePath)) {
1273512738
if (!dryRun) {
12736-
remove(sessionFilePath);
12739+
safeRemove(sessionFilePath);
1273712740
}
1273812741
}
1273912742
}
@@ -12787,7 +12790,7 @@ class SyncEngine {
1278712790
// Cleanup 'resume_download' file
1278812791
if (exists(resumeDownloadFile)) {
1278912792
if (!dryRun) {
12790-
remove(resumeDownloadFile);
12793+
safeRemove(resumeDownloadFile);
1279112794
}
1279212795
}
1279312796
}

src/util.d

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ module util;
33

44
// What does this module require to function?
55
import core.stdc.stdlib: EXIT_SUCCESS, EXIT_FAILURE, exit;
6+
import core.stdc.errno : ENOENT;
67
import std.base64;
78
import std.conv;
89
import std.digest.crc;
@@ -172,7 +173,21 @@ void safeRename(const(char)[] oldPath, const(char)[] newPath, bool dryRun) {
172173

173174
// Deletes the specified file without throwing an exception if it does not exists
174175
void safeRemove(const(char)[] path) {
175-
if (exists(path)) remove(path);
176+
// Set this function name
177+
string thisFunctionName = format("%s.%s", strip(__MODULE__) , strip(getFunctionName!({})));
178+
179+
// Attempt the local deletion
180+
try {
181+
// Attempt once; no pre-check to avoid TOCTTOU
182+
remove(path); // attempt once, no pre-check
183+
return; // removed
184+
} catch (FileException e) {
185+
if (e.errno == ENOENT) { // already gone → fine
186+
return; // nothing to do
187+
}
188+
// Anything else is noteworthy (EISDIR, EACCES, etc.)
189+
displayFileSystemErrorMessage(e.msg, thisFunctionName);
190+
}
176191
}
177192

178193
// Returns the SHA1 hash hex string of a file, or an empty string on failure

0 commit comments

Comments
 (0)