Skip to content

Commit

Permalink
Handle a 'race' condition to process inotify events (#948)
Browse files Browse the repository at this point in the history 
* Handle a 'race' condition to process inotify events generated whilst performing DB or filesystem walk
  • Loading branch information
@abraunegg
abraunegg committed Jun 11, 2020
1 parent 46c91ba commit e321c37
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 19 deletions.
37 changes: 30 additions & 7 deletions src/main.d
View file
Expand Up @@ -734,6 +734,9 @@ int main(string[] args)

// Are we performing a sync, or monitor operation?
if ((cfg.getValueBool("synchronize")) || (cfg.getValueBool("monitor"))) {
// Initialise the monitor class, so that we can do more granular inotify handling when performing the actual sync
// needed for --synchronize and --monitor handling
Monitor m = new Monitor(selectiveSync);

if (cfg.getValueBool("synchronize")) {
if (online) {
Expand All @@ -750,14 +753,14 @@ int main(string[] args)
// perform a --synchronize sync
// fullScanRequired = false, for final true-up
// but if we have sync_list configured, use syncListConfigured which = true
performSync(sync, cfg.getValueString("single_directory"), cfg.getValueBool("download_only"), cfg.getValueBool("local_first"), cfg.getValueBool("upload_only"), LOG_NORMAL, false, syncListConfigured, displaySyncOptions);
performSync(sync, cfg.getValueString("single_directory"), cfg.getValueBool("download_only"), cfg.getValueBool("local_first"), cfg.getValueBool("upload_only"), LOG_NORMAL, false, syncListConfigured, displaySyncOptions, cfg.getValueBool("monitor"), m);
}
}

if (cfg.getValueBool("monitor")) {
log.logAndNotify("Initializing monitor ...");
log.log("OneDrive monitor interval (seconds): ", cfg.getValueLong("monitor_interval"));
Monitor m = new Monitor(selectiveSync);

m.onDirCreated = delegate(string path) {
// Handle .folder creation if skip_dotfiles is enabled
if ((cfg.getValueBool("skip_dotfiles")) && (selectiveSync.isDotFile(path))) {
Expand Down Expand Up @@ -925,16 +928,17 @@ int main(string[] args)
try {
// perform a --monitor sync
log.vlog("Starting a sync with OneDrive");
performSync(sync, cfg.getValueString("single_directory"), cfg.getValueBool("download_only"), cfg.getValueBool("local_first"), cfg.getValueBool("upload_only"), (logMonitorCounter == logInterval ? MONITOR_LOG_QUIET : MONITOR_LOG_SILENT), fullScanRequired, syncListConfiguredFullScanOverride, displaySyncOptions);
performSync(sync, cfg.getValueString("single_directory"), cfg.getValueBool("download_only"), cfg.getValueBool("local_first"), cfg.getValueBool("upload_only"), (logMonitorCounter == logInterval ? MONITOR_LOG_QUIET : MONITOR_LOG_SILENT), fullScanRequired, syncListConfiguredFullScanOverride, displaySyncOptions, cfg.getValueBool("monitor"), m);
if (!cfg.getValueBool("download_only")) {
// discard all events that may have been generated by the sync
// discard all events that may have been generated by the sync that have not already been handled
try {
m.update(false);
} catch (MonitorException e) {
// Catch any exceptions thrown by inotify / monitor engine
log.error("ERROR: The following inotify error was generated: ", e.msg);
}
}
log.vlog("Sync with OneDrive is complete");
} catch (CurlException e) {
// we already tried three times in the performSync routine
// if we still have problems, then the sync handle might have
Expand All @@ -946,7 +950,6 @@ int main(string[] args)
log.log("Cannot initialize connection to OneDrive");
}
// performSync complete, set lastCheckTime to current time
log.vlog("Sync with OneDrive is complete");
fullScanRequired = false;
if (syncListConfigured) {
syncListConfiguredFullScanOverride = false;
Expand Down Expand Up @@ -1016,7 +1019,7 @@ bool initSyncEngine(SyncEngine sync)
}

// try to synchronize the folder three times
void performSync(SyncEngine sync, string singleDirectory, bool downloadOnly, bool localFirst, bool uploadOnly, long logLevel, bool fullScanRequired, bool syncListConfiguredFullScanOverride, bool displaySyncOptions)
void performSync(SyncEngine sync, string singleDirectory, bool downloadOnly, bool localFirst, bool uploadOnly, long logLevel, bool fullScanRequired, bool syncListConfiguredFullScanOverride, bool displaySyncOptions, bool monitorEnabled, Monitor m)
{
int count;
string remotePath = "/";
Expand Down Expand Up @@ -1131,7 +1134,27 @@ void performSync(SyncEngine sync, string singleDirectory, bool downloadOnly, boo
log.vdebug(logOutputMessage);
log.vdebug(syncCallLogOutput);
}
sync.scanForDifferences(localPath);

// What sort of local scan do we want to do?
// In --monitor mode, when performing the DB scan, a race condition occurs where by if a file or folder is moved during this process
// the inotify event is discarded once performSync() is finished (see m.update(false) above), so these events need to be handled
// This can be remediated by breaking the DB and file system scan into separate processes, and handing any applicable inotify events in between
if (!monitorEnabled) {
// --synchronize in use
// standard process flow
sync.scanForDifferences(localPath);
} else {
// --monitor in use
// Use individual calls with inotify checks between to avoid a race condition between these 2 functions
// Database scan
sync.scanForDifferencesDatabaseScan(localPath);
// handle any inotify events that occured 'whilst' we were scanning the database
m.update(true);
// Filesystem walk to find new files not uploaded
sync.scanForDifferencesFilesystemScan(localPath);
// handle any inotify events that occured 'whilst' we were scanning the local filesystem
m.update(true);
}

// At this point, all OneDrive changes / local changes should be uploaded and in sync
// This MAY not be the case when using sync_list, thus a full walk of OneDrive ojects is required
Expand Down
57 changes: 45 additions & 12 deletions src/sync.d
View file
Expand Up @@ -2243,19 +2243,40 @@ final class SyncEngine
log.vlog("Uploading differences of ", path);
Item item;
if (itemdb.selectByPath(path, defaultDriveId, item)) {
// Database scan of every item in DB, does it still exist on disk in the location the DB thinks it is
uploadDifferences(item);
}

log.vlog("Uploading new items of ", path);
// Filesystem walk to find new files not uploaded
uploadNewItems(path);

// clean up idsToDelete only if --dry-run is set
if (dryRun) {
idsToDelete.length = 0;
assumeSafeAppend(idsToDelete);
}
}


// scan the given directory for differences only - for use with --monitor
void scanForDifferencesDatabaseScan(const(string) path)
{
// scan for changes in the path provided
log.vlog("Uploading differences of ", path);
Item item;
if (itemdb.selectByPath(path, defaultDriveId, item)) {
// Database scan of every item in DB, does it still exist on disk in the location the DB thinks it is
uploadDifferences(item);
}
}

// scan the given directory for new items - for use with --monitor
void scanForDifferencesFilesystemScan(const(string) path)
{
log.vlog("Uploading new items of ", path);
// Filesystem walk to find new files not uploaded
uploadNewItems(path);
}

private void uploadDifferences(const ref Item item)
{
// see if this item.id we were supposed to have deleted
Expand Down Expand Up @@ -2342,12 +2363,19 @@ final class SyncEngine
// Directory does not exist locally
// If we are in a --dry-run situation - this directory may never have existed as we never downloaded it
if (!dryRun) {
log.vlog("The directory has been deleted locally");
if (noRemoteDelete) {
// do not process remote directory delete
log.vlog("Skipping remote directory delete as --upload-only & --no-remote-delete configured");
// Not --dry-run situation
if (!cfg.getValueBool("monitor")) {
// Not in --monitor mode
log.vlog("The directory has been deleted locally");
if (noRemoteDelete) {
// do not process remote directory delete
log.vlog("Skipping remote directory delete as --upload-only & --no-remote-delete configured");
} else {
uploadDeleteItem(item, path);
}
} else {
uploadDeleteItem(item, path);
// Appropriate message as we are in --monitor mode
log.vlog("The directory appears to have been deleted locally .. but we are running in --monitor mode. This may have been 'moved' rather than 'deleted'");
}
} else {
// we are in a --dry-run situation, directory appears to have deleted locally - this directory may never have existed as we never downloaded it ..
Expand Down Expand Up @@ -2750,12 +2778,17 @@ final class SyncEngine
// If we are in a --dry-run situation - this file may never have existed as we never downloaded it
if (!dryRun) {
// Not --dry-run situation
log.vlog("The file has been deleted locally");
if (noRemoteDelete) {
// do not process remote file delete
log.vlog("Skipping remote file delete as --upload-only & --no-remote-delete configured");
if (!cfg.getValueBool("monitor")) {
log.vlog("The file has been deleted locally");
if (noRemoteDelete) {
// do not process remote file delete
log.vlog("Skipping remote file delete as --upload-only & --no-remote-delete configured");
} else {
uploadDeleteItem(item, path);
}
} else {
uploadDeleteItem(item, path);
// Appropriate message as we are in --monitor mode
log.vlog("The file appears to have been deleted locally .. but we are running in --monitor mode. This may have been 'moved' rather than 'deleted'");
}
} else {
// We are in a --dry-run situation, file appears to have deleted locally - this file may never have existed as we never downloaded it ..
Expand Down

0 comments on commit e321c37

Please sign in to comment.