From fe2368c38cc8ed57dbf7fb2614ca2d7939262818 Mon Sep 17 00:00:00 2001 From: Christopher Tate Date: Wed, 17 May 2017 15:42:35 -0700 Subject: [PATCH] Refresh in-memory SharedPreferences instances after restore Existing instances don't know that the file has changed out from under them, so they continue to return stale values from reads, and risk overwriting restored data with stale content if writes are performed. We now tell the backing cache system to induce a reload after restore (i.e. after we might have written a relevant file out from under it). Along the way we shook out an irregularity in the way we were setting up the context topology of non-lifecycle instances of the metadata-handling BackupAgent subclass, so that's fixed now too. Bug 12061817 Test: cts-tradefed run cts -m CtsBackupHostTestCases Change-Id: I401fe9297235b55d8a8f041e430d122dc6e24129 --- core/java/android/app/ContextImpl.java | 21 +++++++++++++ core/java/android/app/backup/BackupAgent.java | 11 +++++-- core/java/android/content/Context.java | 3 ++ core/java/android/content/ContextWrapper.java | 7 ++++- .../server/backup/BackupManagerService.java | 30 ++++++++++++++++--- .../src/android/test/mock/MockContext.java | 6 ++++ 6 files changed, 71 insertions(+), 7 deletions(-) diff --git a/core/java/android/app/ContextImpl.java b/core/java/android/app/ContextImpl.java index 23ec260965d57..8c64129985722 100644 --- a/core/java/android/app/ContextImpl.java +++ b/core/java/android/app/ContextImpl.java @@ -89,6 +89,7 @@ import java.io.FilenameFilter; import java.io.IOException; import java.io.InputStream; import java.nio.ByteOrder; +import java.util.ArrayList; import java.util.Objects; class ReceiverRestrictedContext extends ContextWrapper { @@ -417,6 +418,26 @@ class ContextImpl extends Context { return packagePrefs; } + @Override + public void reloadSharedPreferences() { + // Build the list of all per-context impls (i.e. caches) we know about + ArrayList spImpls = new ArrayList<>(); + synchronized (ContextImpl.class) { + final ArrayMap cache = getSharedPreferencesCacheLocked(); + for (int i = 0; i < cache.size(); i++) { + final SharedPreferencesImpl sp = cache.valueAt(i); + if (sp != null) { + spImpls.add(sp); + } + } + } + + // Issue the reload outside the cache lock + for (int i = 0; i < spImpls.size(); i++) { + spImpls.get(i).startReloadIfChangedUnexpectedly(); + } + } + /** * Try our best to migrate all files from source to target that match * requested prefix. diff --git a/core/java/android/app/backup/BackupAgent.java b/core/java/android/app/backup/BackupAgent.java index 5f92af9fb8daa..42e6147203847 100644 --- a/core/java/android/app/backup/BackupAgent.java +++ b/core/java/android/app/backup/BackupAgent.java @@ -942,6 +942,11 @@ public abstract class BackupAgent extends ContextWrapper { long ident = Binder.clearCallingIdentity(); if (DEBUG) Log.v(TAG, "doRestore() invoked"); + + // Ensure that any side-effect SharedPreferences writes have landed *before* + // we may be about to rewrite the file out from underneath + waitForSharedPrefs(); + BackupDataInput input = new BackupDataInput(data.getFileDescriptor()); try { BackupAgent.this.onRestore(input, appVersionCode, newState); @@ -952,8 +957,8 @@ public abstract class BackupAgent extends ContextWrapper { Log.d(TAG, "onRestore (" + BackupAgent.this.getClass().getName() + ") threw", ex); throw ex; } finally { - // Ensure that any side-effect SharedPreferences writes have landed - waitForSharedPrefs(); + // And bring live SharedPreferences instances up to date + reloadSharedPreferences(); Binder.restoreCallingIdentity(ident); try { @@ -1053,6 +1058,8 @@ public abstract class BackupAgent extends ContextWrapper { } finally { // Ensure that any side-effect SharedPreferences writes have landed waitForSharedPrefs(); + // And bring live SharedPreferences instances up to date + reloadSharedPreferences(); Binder.restoreCallingIdentity(ident); try { diff --git a/core/java/android/content/Context.java b/core/java/android/content/Context.java index 61a7be508716a..9ad33ee0379cf 100644 --- a/core/java/android/content/Context.java +++ b/core/java/android/content/Context.java @@ -813,6 +813,9 @@ public abstract class Context { */ public abstract boolean deleteSharedPreferences(String name); + /** @hide */ + public abstract void reloadSharedPreferences(); + /** * Open a private file associated with this Context's application package * for reading. diff --git a/core/java/android/content/ContextWrapper.java b/core/java/android/content/ContextWrapper.java index e127ca319bd82..20fafb2857f0f 100644 --- a/core/java/android/content/ContextWrapper.java +++ b/core/java/android/content/ContextWrapper.java @@ -19,7 +19,6 @@ package android.content; import android.annotation.SystemApi; import android.app.IApplicationThread; import android.app.IServiceConnection; -import android.app.Notification; import android.content.pm.ApplicationInfo; import android.content.pm.PackageManager; import android.content.res.AssetManager; @@ -173,6 +172,12 @@ public class ContextWrapper extends Context { return mBase.getSharedPreferences(file, mode); } + /** @hide */ + @Override + public void reloadSharedPreferences() { + mBase.reloadSharedPreferences(); + } + @Override public boolean moveSharedPreferencesFrom(Context sourceContext, String name) { return mBase.moveSharedPreferencesFrom(sourceContext, name); diff --git a/services/backup/java/com/android/server/backup/BackupManagerService.java b/services/backup/java/com/android/server/backup/BackupManagerService.java index 38db8dea3a9dd..f1f8757824161 100644 --- a/services/backup/java/com/android/server/backup/BackupManagerService.java +++ b/services/backup/java/com/android/server/backup/BackupManagerService.java @@ -839,6 +839,29 @@ public class BackupManagerService implements BackupManagerServiceInterface { return !appGetsFullBackup(pkg); } + /* + * Construct a backup agent instance for the metadata pseudopackage. This is a + * process-local non-lifecycle agent instance, so we manually set up the context + * topology for it. + */ + PackageManagerBackupAgent makeMetadataAgent() { + PackageManagerBackupAgent pmAgent = new PackageManagerBackupAgent(mPackageManager); + pmAgent.attach(mContext); + pmAgent.onCreate(); + return pmAgent; + } + + /* + * Same as above but with the explicit package-set configuration. + */ + PackageManagerBackupAgent makeMetadataAgent(List packages) { + PackageManagerBackupAgent pmAgent = + new PackageManagerBackupAgent(mPackageManager, packages); + pmAgent.attach(mContext); + pmAgent.onCreate(); + return pmAgent; + } + // ----- Asynchronous backup/restore handler thread ----- private class BackupHandler extends Handler { @@ -2893,8 +2916,7 @@ public class BackupManagerService implements BackupManagerServiceInterface { // because it's cheap and this way we guarantee that we don't get out of // step even if we're selecting among various transports at run time. if (mStatus == BackupTransport.TRANSPORT_OK) { - PackageManagerBackupAgent pmAgent = new PackageManagerBackupAgent( - mPackageManager); + PackageManagerBackupAgent pmAgent = makeMetadataAgent(); mStatus = invokeAgentForBackup(PACKAGE_MANAGER_SENTINEL, IBackupAgent.Stub.asInterface(pmAgent.onBind()), mTransport); addBackupTrace("PMBA invoke: " + mStatus); @@ -7155,7 +7177,7 @@ if (MORE_DEBUG) Slog.v(TAG, " + got " + nRead + "; now wanting " + (size - soF mObserver = observer; mLatchObject = latch; mAgent = null; - mPackageManagerBackupAgent = new PackageManagerBackupAgent(mPackageManager); + mPackageManagerBackupAgent = makeMetadataAgent(); mAgentPackage = null; mTargetApp = null; mObbConnection = new FullBackupObbConnection(); @@ -8817,7 +8839,7 @@ if (MORE_DEBUG) Slog.v(TAG, " + got " + nRead + "; now wanting " + (size - soF // Pull the Package Manager metadata from the restore set first mCurrentPackage = new PackageInfo(); mCurrentPackage.packageName = PACKAGE_MANAGER_SENTINEL; - mPmAgent = new PackageManagerBackupAgent(mPackageManager, null); + mPmAgent = makeMetadataAgent(null); mAgent = IBackupAgent.Stub.asInterface(mPmAgent.onBind()); if (MORE_DEBUG) { Slog.v(TAG, "initiating restore for PMBA"); diff --git a/test-runner/src/android/test/mock/MockContext.java b/test-runner/src/android/test/mock/MockContext.java index bfc2d728ad5c4..ebad81cdda348 100644 --- a/test-runner/src/android/test/mock/MockContext.java +++ b/test-runner/src/android/test/mock/MockContext.java @@ -149,6 +149,12 @@ public class MockContext extends Context { throw new UnsupportedOperationException(); } + /** @hide */ + @Override + public void reloadSharedPreferences() { + throw new UnsupportedOperationException(); + } + @Override public boolean moveSharedPreferencesFrom(Context sourceContext, String name) { throw new UnsupportedOperationException();