Handle a 'race' condition to process inotify events (#948)

* Handle a 'race' condition to process inotify events generated whilst performing DB or filesystem walk
This commit is contained in:
abraunegg 2020-06-12 06:46:59 +10:00 committed by GitHub
parent 46c91bac6e
commit e321c373fd
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 75 additions and 19 deletions

View file

@ -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) {
@ -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))) {
@ -925,9 +928,9 @@ 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) {
@ -935,6 +938,7 @@ int main(string[] args)
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
@ -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;
@ -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 = "/";
@ -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

View file

@ -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
@ -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 ..
@ -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 ..