From 475293a116b5c3f7937bc5075921a2a0cb9c9a29 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Wed, 20 Aug 2014 19:59:45 +0200 Subject: consolidate: prevent concurrent calls of step 2 --- .../keychain/KeychainApplication.java | 13 +- .../keychain/provider/ProviderHelper.java | 189 +++++++++++---------- .../keychain/service/OperationResultParcel.java | 1 + OpenKeychain/src/main/res/values/strings.xml | 3 +- 4 files changed, 115 insertions(+), 91 deletions(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/KeychainApplication.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/KeychainApplication.java index 964a15d58..ead29e229 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/KeychainApplication.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/KeychainApplication.java @@ -86,21 +86,26 @@ public class KeychainApplication extends Application { setupAccountAsNeeded(this); // Update keyserver list as needed - Preferences prefs = Preferences.getPreferences(this); - - prefs.updatePreferences(); + Preferences.getPreferences(this).updatePreferences(); TlsHelper.addStaticCA("pool.sks-keyservers.net", getAssets(), "sks-keyservers.netCA.cer"); TemporaryStorageProvider.cleanUp(this); + checkConsolidateRecovery(); + + } + + public void checkConsolidateRecovery() { + // restart consolidate process if it has been interruped before - if (prefs.getCachedConsolidate()) { + if (Preferences.getPreferences(this).getCachedConsolidate()) { // do something which calls ProviderHelper.consolidateDatabaseStep2 with a progressable Intent consolidateIntent = new Intent(this, ConsolidateDialogActivity.class); consolidateIntent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK); startActivity(consolidateIntent); } + } public static void setupAccountAsNeeded(Context context) { diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java index 125f33ded..0d6e83749 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java @@ -961,111 +961,128 @@ public class ProviderHelper { return consolidateDatabaseStep2(progress, true); } + private static boolean mConsolidateCritical = false; + private ConsolidateResult consolidateDatabaseStep2(Progressable progress, boolean recovery) { - Preferences prefs = Preferences.getPreferences(mContext); + synchronized (ProviderHelper.class) { + if (mConsolidateCritical) { + log(LogLevel.ERROR, LogType.MSG_CON_ERROR_CONCURRENT); + return new ConsolidateResult(ConsolidateResult.RESULT_ERROR, mLog); + } + mConsolidateCritical = true; + } - // Set flag that we have a cached consolidation here - int numSecrets = prefs.getCachedConsolidateNumSecrets(); - int numPublics = prefs.getCachedConsolidateNumPublics(); + try { + Preferences prefs = Preferences.getPreferences(mContext); - if (recovery) { - if (numSecrets >= 0 && numPublics >= 0) { - log(LogLevel.START, LogType.MSG_CON_RECOVER, numSecrets, numPublics); - } else { - log(LogLevel.START, LogType.MSG_CON_RECOVER_UNKNOWN); + // Set flag that we have a cached consolidation here + int numSecrets = prefs.getCachedConsolidateNumSecrets(); + int numPublics = prefs.getCachedConsolidateNumPublics(); + + if (recovery) { + if (numSecrets >= 0 && numPublics >= 0) { + log(LogLevel.START, LogType.MSG_CON_RECOVER, numSecrets, numPublics); + } else { + log(LogLevel.START, LogType.MSG_CON_RECOVER_UNKNOWN); + } + mIndent += 1; } - mIndent += 1; - } - if ( ! prefs.getCachedConsolidate()) { - log(LogLevel.ERROR, LogType.MSG_CON_ERROR_BAD_STATE); - return new ConsolidateResult(ConsolidateResult.RESULT_ERROR, mLog); - } + if (!prefs.getCachedConsolidate()) { + log(LogLevel.ERROR, LogType.MSG_CON_ERROR_BAD_STATE); + return new ConsolidateResult(ConsolidateResult.RESULT_ERROR, mLog); + } - // 2. wipe database (IT'S DANGEROUS) - log(LogLevel.DEBUG, LogType.MSG_CON_DB_CLEAR); - mContentResolver.delete(KeyRings.buildUnifiedKeyRingsUri(), null, null); + // 2. wipe database (IT'S DANGEROUS) + log(LogLevel.DEBUG, LogType.MSG_CON_DB_CLEAR); + mContentResolver.delete(KeyRings.buildUnifiedKeyRingsUri(), null, null); - // debug: break if this isn't recovery - if (!recovery) { - return new ConsolidateResult(ConsolidateResult.RESULT_ERROR, mLog); - } + // debug: break if this isn't recovery + if (!recovery) { + return new ConsolidateResult(ConsolidateResult.RESULT_ERROR, mLog); + } - FileImportCache cacheSecret = - new FileImportCache(mContext, "consolidate_secret.pcl"); - FileImportCache cachePublic = - new FileImportCache(mContext, "consolidate_public.pcl"); + FileImportCache cacheSecret = + new FileImportCache(mContext, "consolidate_secret.pcl"); + FileImportCache cachePublic = + new FileImportCache(mContext, "consolidate_public.pcl"); - // 3. Re-Import secret keyrings from cache - if (numSecrets > 0) try { - log(LogLevel.DEBUG, LogType.MSG_CON_REIMPORT_SECRET, numSecrets); - mIndent += 1; + // 3. Re-Import secret keyrings from cache + if (numSecrets > 0) try { + log(LogLevel.DEBUG, LogType.MSG_CON_REIMPORT_SECRET, numSecrets); + mIndent += 1; - new PgpImportExport(mContext, this, - new ProgressFixedScaler(progress, 10, 25, 100, R.string.progress_con_reimport)) - .importKeyRings(cacheSecret.readCache(false), numSecrets); - } catch (IOException e) { - Log.e(Constants.TAG, "error importing secret", e); - log(LogLevel.ERROR, LogType.MSG_CON_ERROR_SECRET); - return new ConsolidateResult(ConsolidateResult.RESULT_ERROR, mLog); - } finally { - mIndent -= 1; - } else { - log(LogLevel.DEBUG, LogType.MSG_CON_REIMPORT_SECRET_SKIP); - } + new PgpImportExport(mContext, this, + new ProgressFixedScaler(progress, 10, 25, 100, R.string.progress_con_reimport)) + .importKeyRings(cacheSecret.readCache(false), numSecrets); + } catch (IOException e) { + Log.e(Constants.TAG, "error importing secret", e); + log(LogLevel.ERROR, LogType.MSG_CON_ERROR_SECRET); + return new ConsolidateResult(ConsolidateResult.RESULT_ERROR, mLog); + } finally { + mIndent -= 1; + } + else { + log(LogLevel.DEBUG, LogType.MSG_CON_REIMPORT_SECRET_SKIP); + } - // 4. Re-Import public keyrings from cache - if (numPublics > 0) try { - log(LogLevel.DEBUG, LogType.MSG_CON_REIMPORT_PUBLIC, numPublics); - mIndent += 1; + // 4. Re-Import public keyrings from cache + if (numPublics > 0) try { + log(LogLevel.DEBUG, LogType.MSG_CON_REIMPORT_PUBLIC, numPublics); + mIndent += 1; - new PgpImportExport(mContext, this, - new ProgressFixedScaler(progress, 25, 99, 100, R.string.progress_con_reimport)) - .importKeyRings(cachePublic.readCache(false), numPublics); - } catch (IOException e) { - Log.e(Constants.TAG, "error importing public", e); - log(LogLevel.ERROR, LogType.MSG_CON_ERROR_PUBLIC); - return new ConsolidateResult(ConsolidateResult.RESULT_ERROR, mLog); - } finally { - mIndent -= 1; - } else { - log(LogLevel.DEBUG, LogType.MSG_CON_REIMPORT_PUBLIC_SKIP); - } + new PgpImportExport(mContext, this, + new ProgressFixedScaler(progress, 25, 99, 100, R.string.progress_con_reimport)) + .importKeyRings(cachePublic.readCache(false), numPublics); + } catch (IOException e) { + Log.e(Constants.TAG, "error importing public", e); + log(LogLevel.ERROR, LogType.MSG_CON_ERROR_PUBLIC); + return new ConsolidateResult(ConsolidateResult.RESULT_ERROR, mLog); + } finally { + mIndent -= 1; + } + else { + log(LogLevel.DEBUG, LogType.MSG_CON_REIMPORT_PUBLIC_SKIP); + } - log(LogLevel.INFO, LogType.MSG_CON_CRITICAL_OUT); - Preferences.getPreferences(mContext).setCachedConsolidate(false); + log(LogLevel.INFO, LogType.MSG_CON_CRITICAL_OUT); + Preferences.getPreferences(mContext).setCachedConsolidate(false); - // 5. Delete caches - try { - log(LogLevel.DEBUG, LogType.MSG_CON_DELETE_SECRET); - mIndent += 1; - cacheSecret.delete(); - } catch (IOException e) { - // doesn't /really/ matter - Log.e(Constants.TAG, "IOException during delete of secret cache", e); - log(LogLevel.WARN, LogType.MSG_CON_WARN_DELETE_SECRET); - } finally { - mIndent -= 1; - } + // 5. Delete caches + try { + log(LogLevel.DEBUG, LogType.MSG_CON_DELETE_SECRET); + mIndent += 1; + cacheSecret.delete(); + } catch (IOException e) { + // doesn't /really/ matter + Log.e(Constants.TAG, "IOException during delete of secret cache", e); + log(LogLevel.WARN, LogType.MSG_CON_WARN_DELETE_SECRET); + } finally { + mIndent -= 1; + } - try { - log(LogLevel.DEBUG, LogType.MSG_CON_DELETE_PUBLIC); - mIndent += 1; - cachePublic.delete(); - } catch (IOException e) { - // doesn't /really/ matter - Log.e(Constants.TAG, "IOException during deletion of public cache", e); - log(LogLevel.WARN, LogType.MSG_CON_WARN_DELETE_PUBLIC); - } finally { + try { + log(LogLevel.DEBUG, LogType.MSG_CON_DELETE_PUBLIC); + mIndent += 1; + cachePublic.delete(); + } catch (IOException e) { + // doesn't /really/ matter + Log.e(Constants.TAG, "IOException during deletion of public cache", e); + log(LogLevel.WARN, LogType.MSG_CON_WARN_DELETE_PUBLIC); + } finally { + mIndent -= 1; + } + + progress.setProgress(100, 100); + log(LogLevel.OK, LogType.MSG_CON_SUCCESS); mIndent -= 1; - } - progress.setProgress(100, 100); - log(LogLevel.OK, LogType.MSG_CON_SUCCESS); - mIndent -= 1; + return new ConsolidateResult(ConsolidateResult.RESULT_OK, mLog); - return new ConsolidateResult(ConsolidateResult.RESULT_OK, mLog); + } finally { + mConsolidateCritical = false; + } } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java index 91939739a..7fe712534 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java @@ -396,6 +396,7 @@ public class OperationResultParcel implements Parcelable { MSG_CON_DELETE_PUBLIC (R.string.msg_con_delete_public), MSG_CON_DELETE_SECRET (R.string.msg_con_delete_secret), MSG_CON_ERROR_BAD_STATE (R.string.msg_con_error_bad_state), + MSG_CON_ERROR_CONCURRENT(R.string.msg_con_error_concurrent), MSG_CON_ERROR_DB (R.string.msg_con_error_db), MSG_CON_ERROR_IO_PUBLIC (R.string.msg_con_error_io_public), MSG_CON_ERROR_IO_SECRET (R.string.msg_con_error_io_secret), diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml index 64c743ead..3175d679b 100644 --- a/OpenKeychain/src/main/res/values/strings.xml +++ b/OpenKeychain/src/main/res/values/strings.xml @@ -673,7 +673,8 @@ Consolidating database - Consolidation started while no database was cached! This is probably a programming error, please file a bug report. + Consolidation was started while no database was cached! This is probably a programming error, please file a bug report. + Consolidation aborted, already running on other thread! Saving secret keyrings Saving public keyrings Clearing database -- cgit v1.2.3