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

diff --git a/core/java/android/app/ContextImpl.java b/core/java/android/app/ContextImpl.java
index 23ec260..8c64129 100644 (file)
--- 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<SharedPreferencesImpl> spImpls = new ArrayList<>();
+        synchronized (ContextImpl.class) {
+            final ArrayMap<File, SharedPreferencesImpl> 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 5f92af9..42e6147 100644 (file)
--- 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 61a7be5..9ad33ee 100644 (file)
--- 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 e127ca3..20fafb2 100644 (file)
--- 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 38db8de..f1f8757 100644 (file)
--- 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<PackageInfo> 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 bfc2d72..ebad81c 100644 (file)
--- 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();