From 0cd9df74151943df4ef22df9ea5c85a8d1a44c59 Mon Sep 17 00:00:00 2001 From: Philipp Crocoll Date: Tue, 3 Jun 2025 15:37:21 +0200 Subject: [PATCH] fix issues with background sync and multiple databases (especially autoopen) --- .../database/SynchronizeCachedDatabase.cs | 16 +++++---- src/Kp2aBusinessLogic/database/edit/LoadDB.cs | 34 ++++++------------- src/Kp2aBusinessLogic/database/edit/SaveDB.cs | 2 +- src/keepass2android-app/GroupBaseActivity.cs | 4 +-- src/keepass2android-app/PasswordActivity.cs | 27 ++++++++++----- src/keepass2android-app/QuickUnlock.cs | 2 +- .../SelectCurrentDbActivity.cs | 26 ++++++++++++++ src/keepass2android-app/SyncUtil.cs | 13 +++---- 8 files changed, 76 insertions(+), 48 deletions(-) diff --git a/src/Kp2aBusinessLogic/database/SynchronizeCachedDatabase.cs b/src/Kp2aBusinessLogic/database/SynchronizeCachedDatabase.cs index ce650a83..82ab2be6 100644 --- a/src/Kp2aBusinessLogic/database/SynchronizeCachedDatabase.cs +++ b/src/Kp2aBusinessLogic/database/SynchronizeCachedDatabase.cs @@ -9,6 +9,7 @@ using KeePassLib.Serialization; using keepass2android.Io; using KeePass.Util; using Group.Pals.Android.Lib.UI.Filechooser.Utils; +using KeePassLib; namespace keepass2android { @@ -16,12 +17,14 @@ namespace keepass2android { private readonly IKp2aApp _app; private IDatabaseModificationWatcher _modificationWatcher; + private readonly Database _database; - public SynchronizeCachedDatabase(IKp2aApp app, OnOperationFinishedHandler operationFinishedHandler, IDatabaseModificationWatcher modificationWatcher) + public SynchronizeCachedDatabase(IKp2aApp app, Database database, OnOperationFinishedHandler operationFinishedHandler, IDatabaseModificationWatcher modificationWatcher) : base(app, operationFinishedHandler) { _app = app; + _database = database; _modificationWatcher = modificationWatcher; } @@ -29,7 +32,7 @@ namespace keepass2android { try { - IOConnectionInfo ioc = _app.CurrentDb.Ioc; + IOConnectionInfo ioc = _database.Ioc; IFileStorage fileStorage = _app.GetFileStorage(ioc); if (!(fileStorage is CachingFileStorage)) { @@ -86,13 +89,13 @@ namespace keepass2android { Finish(true, _app.GetResourceString(UiStringKey.SynchronizedDatabaseSuccessfully)); } - }), _app.CurrentDb, false, remoteData, _modificationWatcher); + }), _database, false, remoteData, _modificationWatcher); _saveDb.SetStatusLogger(StatusLogger); _saveDb.DoNotSetStatusLoggerMessage = true; //Keep "sync db" as main message _saveDb.SyncInBackground = false; _saveDb.Run(); - _app.CurrentDb.UpdateGlobals(); + _database.UpdateGlobals(); _app.MarkAllGroupsAsDirty(); } @@ -109,7 +112,7 @@ namespace keepass2android { new Handler(Looper.MainLooper).Post(() => { - _app.CurrentDb.UpdateGlobals(); + _database.UpdateGlobals(); _app.MarkAllGroupsAsDirty(); Finish(true, _app.GetResourceString(UiStringKey.SynchronizedDatabaseSuccessfully)); @@ -118,10 +121,9 @@ namespace keepass2android } }); var _loadDb = new LoadDb(_app, ioc, Task.FromResult(remoteData), - _app.CurrentDb.KpDatabase.MasterKey, null, onFinished, true, false, _modificationWatcher); + _database.KpDatabase.MasterKey, null, onFinished, true, false, _modificationWatcher); _loadDb.SetStatusLogger(StatusLogger); _loadDb.DoNotSetStatusLoggerMessage = true; //Keep "sync db" as main message - _loadDb.SyncInBackground = false; _loadDb.Run(); } diff --git a/src/Kp2aBusinessLogic/database/edit/LoadDB.cs b/src/Kp2aBusinessLogic/database/edit/LoadDB.cs index eb262952..5c7271e7 100644 --- a/src/Kp2aBusinessLogic/database/edit/LoadDB.cs +++ b/src/Kp2aBusinessLogic/database/edit/LoadDB.cs @@ -41,7 +41,7 @@ namespace keepass2android IDatabaseFormat _format; public bool DoNotSetStatusLoggerMessage = false; - public bool SyncInBackground { get; set; } + public LoadDb(IKp2aApp app, IOConnectionInfo ioc, Task databaseData, CompositeKey compositeKey, string keyfileOrProvider, OnOperationFinishedHandler operationFinishedHandler, @@ -56,7 +56,6 @@ namespace keepass2android _updateLastUsageTimestamp = updateLastUsageTimestamp; _makeCurrent = makeCurrent; _rememberKeyfile = app.GetBooleanPreference(PreferenceKey.remember_keyfile); - SyncInBackground = _app.SyncInBackgroundPreference; } protected bool success = false; @@ -76,7 +75,7 @@ namespace keepass2android var fileStorage = _app.GetFileStorage(_ioc); - bool requiresSubsequentSync = false; + RequiresSubsequentSync = false; if (!DoNotSetStatusLoggerMessage) @@ -100,7 +99,7 @@ namespace keepass2android cachingFileStorage.IsOffline = true; //no warning. We'll trigger a sync later. cachingFileStorage.TriggerWarningWhenFallingBackToCache = false; - requiresSubsequentSync = true; + RequiresSubsequentSync = true; } using (Stream s = fileStorage.OpenFileForRead(_ioc)) @@ -118,7 +117,7 @@ namespace keepass2android //ok, try to load the database. Let's start with Kdbx format and retry later if that is the wrong guess: _format = new KdbxDatabaseFormat(KdbxDatabaseFormat.GetFormatToUse(fileStorage.GetFileExtension(_ioc))); - TryLoad(databaseStream, requiresSubsequentSync); + TryLoad(databaseStream); @@ -171,12 +170,14 @@ namespace keepass2android } - /// + public bool RequiresSubsequentSync { get; set; } = false; + + /// /// Holds the exception which was thrown during execution (if any) /// public Exception Exception { get; set; } - Database TryLoad(MemoryStream databaseStream, bool requiresSubsequentSync) + Database TryLoad(MemoryStream databaseStream) { //create a copy of the stream so we can try again if we get an exception which indicates we should change parameters //This is not optimal in terms of (short-time) memory usage but is hard to avoid because the Keepass library closes streams also in case of errors. @@ -197,27 +198,14 @@ namespace keepass2android Database newDb = _app.LoadDatabase(_ioc, workingCopy, _compositeKey, StatusLogger, _format, _makeCurrent, _modificationWatcher); Kp2aLog.Log("LoadDB OK"); - if (requiresSubsequentSync) - { - var syncTask = new SynchronizeCachedDatabase(_app, new ActionOnOperationFinished(_app, - (success, message, context) => - { - if (!String.IsNullOrEmpty(message)) - _app.ShowMessage(context, message, - success ? MessageSeverity.Info : MessageSeverity.Error); - - }), new BackgroundDatabaseModificationLocker(_app) - ); - OperationRunner.Instance.Run(_app, syncTask); - } - + Finish(true, _format.SuccessMessage); return newDb; } catch (OldFormatException) { _format = new KdbDatabaseFormat(_app); - return TryLoad(databaseStream, requiresSubsequentSync); + return TryLoad(databaseStream); } catch (InvalidCompositeKeyException) { @@ -229,7 +217,7 @@ namespace keepass2android //retry without password: _compositeKey.RemoveUserKey(passwordKey); //retry: - return TryLoad(databaseStream, requiresSubsequentSync); + return TryLoad(databaseStream); } else throw; } diff --git a/src/Kp2aBusinessLogic/database/edit/SaveDB.cs b/src/Kp2aBusinessLogic/database/edit/SaveDB.cs index 6df6e721..81615a46 100644 --- a/src/Kp2aBusinessLogic/database/edit/SaveDB.cs +++ b/src/Kp2aBusinessLogic/database/edit/SaveDB.cs @@ -235,7 +235,7 @@ namespace keepass2android { if (requiresSubsequentSync) { - var syncTask = new SynchronizeCachedDatabase(_app, new ActionOnOperationFinished(_app, + var syncTask = new SynchronizeCachedDatabase(_app, _db, new ActionOnOperationFinished(_app, (success, message, context) => { if (!System.String.IsNullOrEmpty(message)) diff --git a/src/keepass2android-app/GroupBaseActivity.cs b/src/keepass2android-app/GroupBaseActivity.cs index ffe8909b..73deb330 100644 --- a/src/keepass2android-app/GroupBaseActivity.cs +++ b/src/keepass2android-app/GroupBaseActivity.cs @@ -1222,7 +1222,7 @@ namespace keepass2android return true; case Resource.Id.menu_sync: - new SyncUtil(this).StartSynchronizeDatabase(); + new SyncUtil(this).StartSynchronizeDatabase(App.Kp2a.CurrentDb.Ioc); return true; case Resource.Id.menu_work_offline: @@ -1233,7 +1233,7 @@ namespace keepass2android case Resource.Id.menu_work_online: App.Kp2a.OfflineMode = App.Kp2a.OfflineModePreference = false; UpdateOfflineModeMenu(); - new SyncUtil(this).StartSynchronizeDatabase(); + new SyncUtil(this).StartSynchronizeDatabase(App.Kp2a.CurrentDb.Ioc); return true; case Resource.Id.menu_open_other_db: AppTask.SetActivityResult(this, KeePass.ExitLoadAnotherDb); diff --git a/src/keepass2android-app/PasswordActivity.cs b/src/keepass2android-app/PasswordActivity.cs index fc6414fd..b390b886 100644 --- a/src/keepass2android-app/PasswordActivity.cs +++ b/src/keepass2android-app/PasswordActivity.cs @@ -222,6 +222,7 @@ namespace keepass2android //StackBaseActivity will launch the next activity Intent data = new Intent(); data.PutExtra("ioc", IOConnectionInfo.SerializeToString(_ioConnection)); + data.PutExtra("requiresSubsequentSync", _lastLoadOperation?.RequiresSubsequentSync == true); SetResult(Result.Ok, data); @@ -1441,14 +1442,21 @@ namespace keepass2android Handler handler = new Handler(); OnOperationFinishedHandler onOperationFinishedHandler = new AfterLoad(handler, this, _ioConnection); - LoadDb task = (KeyProviderTypes.Contains(KeyProviders.Otp)) + LoadDb loadOperation = (KeyProviderTypes.Contains(KeyProviders.Otp)) ? new SaveOtpAuxFileAndLoadDb(App.Kp2a, _ioConnection, _loadDbFileTask, compositeKey, GetKeyProviderString(), onOperationFinishedHandler, this, true, _makeCurrent) : new LoadDb(App.Kp2a, _ioConnection, _loadDbFileTask, compositeKey, GetKeyProviderString(), onOperationFinishedHandler,true, _makeCurrent); _loadDbFileTask = null; // prevent accidental re-use - new BlockingOperationStarter(App.Kp2a, task).Run(); - } + _lastLoadOperation = loadOperation; + + + //Don't use BlockingOperationStarter as that would cancel running operations. + //This is bad when used with AutoOpen: we might get here when one database has loaded and is now synced in the background. + //We don't want to cancel this. + OperationRunner.Instance.Run(App.Kp2a, loadOperation, true); + + } catch (Exception e) { Kp2aLog.LogUnexpectedError(new Exception("cannot load database: "+e + ", c: " + (compositeKey != null) + (_ioConnection != null) + (_keyFile != null), e)); @@ -1572,7 +1580,9 @@ namespace keepass2android } private bool hasRequestedKeyboardActivation = false; - protected override void OnStart() + private LoadDb _lastLoadOperation; + + protected override void OnStart() { base.OnStart(); _starting = true; @@ -1886,11 +1896,12 @@ namespace keepass2android Handler handler = new Handler(); OnOperationFinishedHandler onOperationFinishedHandler = new AfterLoad(handler, this, _ioConnection); _performingLoad = true; - LoadDb task = new LoadDb(App.Kp2a, _ioConnection, _loadDbFileTask, compositeKeyForImmediateLoad, GetKeyProviderString(), + LoadDb loadOperation = new LoadDb(App.Kp2a, _ioConnection, _loadDbFileTask, compositeKeyForImmediateLoad, GetKeyProviderString(), onOperationFinishedHandler, false, _makeCurrent); _loadDbFileTask = null; // prevent accidental re-use - new BlockingOperationStarter(App.Kp2a, task).Run(); - compositeKeyForImmediateLoad = null; //don't reuse or keep in memory + _lastLoadOperation = loadOperation; + OperationRunner.Instance.Run(App.Kp2a, loadOperation, true); + compositeKeyForImmediateLoad = null; //don't reuse or keep in memory } else @@ -2242,7 +2253,7 @@ namespace keepass2android if (!Success) _act.InitFingerprintUnlock(); - + _act._lastLoadOperation = null; _act._performingLoad = false; } diff --git a/src/keepass2android-app/QuickUnlock.cs b/src/keepass2android-app/QuickUnlock.cs index 56acd178..f8b265c9 100644 --- a/src/keepass2android-app/QuickUnlock.cs +++ b/src/keepass2android-app/QuickUnlock.cs @@ -339,7 +339,7 @@ namespace keepass2android if (PreferenceManager.GetDefaultSharedPreferences(this) .GetBoolean(GetString(Resource.String.SyncAfterQuickUnlock_key), false)) { - new SyncUtil(this).StartSynchronizeDatabase(); + new SyncUtil(this).StartSynchronizeDatabase(App.Kp2a.CurrentDb.Ioc); } Finish(); diff --git a/src/keepass2android-app/SelectCurrentDbActivity.cs b/src/keepass2android-app/SelectCurrentDbActivity.cs index 6be80475..756802fc 100644 --- a/src/keepass2android-app/SelectCurrentDbActivity.cs +++ b/src/keepass2android-app/SelectCurrentDbActivity.cs @@ -558,6 +558,7 @@ namespace keepass2android private OpenDatabaseAdapter _adapter; private MyBroadcastReceiver _intentReceiver; private bool _isForeground; + private readonly List _pendingBackgroundSyncs = new List(); public override void OnBackPressed() { @@ -598,11 +599,36 @@ namespace keepass2android string iocString = data?.GetStringExtra("ioc"); IOConnectionInfo ioc = IOConnectionInfo.UnserializeFromString(iocString); + + //we first store the required sync operation and delay its execution until we loaded all AutoOpen entries (from local file) + //if required + bool requiresSubsequentSync = data?.GetBooleanExtra("requiresSubsequentSync", false) ?? false; + if (requiresSubsequentSync) + { + _pendingBackgroundSyncs.Add(ioc); + } + if (App.Kp2a.TrySelectCurrentDb(ioc)) { if (OpenAutoExecEntries(App.Kp2a.CurrentDb)) return; LaunchingOther = true; AppTask.CanActivateSearchViewOnStart = true; + + foreach (var pendingSyncIoc in _pendingBackgroundSyncs) + { + try + { + new SyncUtil(this).StartSynchronizeDatabase(pendingSyncIoc); + } + catch (Exception e) + { + App.Kp2a.ShowMessage(this, "Failed to synchronize database", MessageSeverity.Error); + Kp2aLog.LogUnexpectedError(e); + } + } + + _pendingBackgroundSyncs.Clear(); + AppTask.LaunchFirstGroupActivity(this); } diff --git a/src/keepass2android-app/SyncUtil.cs b/src/keepass2android-app/SyncUtil.cs index 60b1c78d..adc56e88 100644 --- a/src/keepass2android-app/SyncUtil.cs +++ b/src/keepass2android-app/SyncUtil.cs @@ -50,9 +50,11 @@ namespace keepass2android } - public void StartSynchronizeDatabase() + public void StartSynchronizeDatabase(IOConnectionInfo ioc) { - var filestorage = App.Kp2a.GetFileStorage(App.Kp2a.CurrentDb.Ioc); + var filestorage = App.Kp2a.GetFileStorage(ioc); + var databaseForIoc = App.Kp2a.GetDatabase(ioc); + OperationWithFinishHandler task; OnOperationFinishedHandler onOperationFinishedHandler = new ActionInContextInstanceOnOperationFinished(_activity.ContextInstanceId, App.Kp2a, (success, message, context) => { @@ -67,9 +69,9 @@ namespace keepass2android adapter?.NotifyDataSetChanged(); }); - if (App.Kp2a.CurrentDb?.OtpAuxFileIoc != null) + if (databaseForIoc?.OtpAuxFileIoc != null) { - var task2 = new SyncOtpAuxFile(_activity, App.Kp2a.CurrentDb.OtpAuxFileIoc); + var task2 = new SyncOtpAuxFile(_activity, databaseForIoc.OtpAuxFileIoc); OperationRunner.Instance.Run(App.Kp2a, task2); } @@ -79,11 +81,10 @@ namespace keepass2android if (filestorage is CachingFileStorage) { - task = new SynchronizeCachedDatabase(App.Kp2a, onOperationFinishedHandler, new BackgroundDatabaseModificationLocker(App.Kp2a)); + task = new SynchronizeCachedDatabase(App.Kp2a, databaseForIoc, onOperationFinishedHandler, new BackgroundDatabaseModificationLocker(App.Kp2a)); } else { - //TODO do we want this to run in the background? task = new CheckDatabaseForChanges( App.Kp2a, onOperationFinishedHandler); }