[SettingsProvider] skip caching in system server

The old implementation avoids closing the memory int array if the
process is the system process. However, this doesn't prevent the object
to be garbage collected. As a result, the array may be left open without
being properly closed.

This change skips sending the memory int array and index if the query
is from inside system server; should also addresses StrictMode complaints.

BUG: 277448672
Test: boots
Change-Id: I309ad3558a77d8e7d0c8a8d2c8f45db5a316c976

1 files changed, 56 insertions, 44 deletions

diff --git a/core/java/android/provider/Settings.java b/core/java/android/provider/Settings.java
index 2efb26593780..49425452c3aa 100644
--- a/core/java/android/provider/Settings.java
+++ b/core/java/android/provider/Settings.java
@@ -3036,9 +3036,7 @@ public final class Settings {
 
         public void destroy() {
             try {
-                // If this process is the system server process, mArray is the same object as
-                // the memory int array kept inside SettingsProvider, so skipping the close()
-                if (!Settings.isInSystemServer() && !mArray.isClosed()) {
+                if (!mArray.isClosed()) {
                     mArray.close();
                 }
             } catch (IOException e) {
@@ -3218,8 +3216,9 @@ public final class Settings {
         @UnsupportedAppUsage
         public String getStringForUser(ContentResolver cr, String name, final int userHandle) {
             final boolean isSelf = (userHandle == UserHandle.myUserId());
+            final boolean useCache = isSelf && !isInSystemServer();
             boolean needsGenerationTracker = false;
-            if (isSelf) {
+            if (useCache) {
                 synchronized (NameValueCache.this) {
                     final GenerationTracker generationTracker = mGenerationTrackers.get(name);
                     if (generationTracker != null) {
@@ -3251,8 +3250,10 @@ public final class Settings {
                 needsGenerationTracker = true;
             } else {
                 if (DEBUG || LOCAL_LOGV) {
-                    Log.v(TAG, "get setting for user " + userHandle
-                            + " by user " + UserHandle.myUserId() + " so skipping cache");
+                    Log.v(TAG, "get setting " + name + " for user " + userHandle + " by user "
+                            + UserHandle.myUserId()
+                            + (isInSystemServer() ? " in system_server" : "")
+                            + " so skipping cache");
                 }
             }
 
@@ -3365,9 +3366,12 @@ public final class Settings {
                                 }
                             }
                         } else {
-                            if (LOCAL_LOGV) Log.i(TAG, "call-query of user " + userHandle
-                                    + " by " + UserHandle.myUserId()
-                                    + " so not updating cache");
+                            if (DEBUG || LOCAL_LOGV) {
+                                Log.i(TAG, "call-query of user " + userHandle
+                                        + " by " + UserHandle.myUserId()
+                                        + (isInSystemServer() ? " in system_server" : "")
+                                        + " so not updating cache");
+                            }
                         }
                         return value;
                     }
@@ -3440,55 +3444,63 @@ public final class Settings {
 
         public ArrayMap<String, String> getStringsForPrefix(ContentResolver cr, String prefix,
                 List<String> names) {
+            final boolean useCache = !isInSystemServer();
             String namespace = prefix.substring(0, prefix.length() - 1);
             Config.enforceReadPermission(namespace);
             ArrayMap<String, String> keyValues = new ArrayMap<>();
             int currentGeneration = -1;
             boolean needsGenerationTracker = false;
 
-            synchronized (NameValueCache.this) {
-                final GenerationTracker generationTracker = mGenerationTrackers.get(prefix);
-                if (generationTracker != null) {
-                    if (generationTracker.isGenerationChanged()) {
-                        if (DEBUG) {
-                            Log.i(TAG, "Generation changed for prefix:" + prefix
-                                    + " type:" + mUri.getPath()
-                                    + " in package:" + cr.getPackageName());
-                        }
-                        for (int i = mValues.size() - 1; i >= 0; i--) {
-                            String key = mValues.keyAt(i);
-                            if (key.startsWith(prefix)) {
-                                mValues.remove(key);
-                            }
-                        }
-                    } else {
-                        boolean prefixCached = mValues.containsKey(prefix);
-                        if (prefixCached) {
+            if (useCache) {
+                synchronized (NameValueCache.this) {
+                    final GenerationTracker generationTracker = mGenerationTrackers.get(prefix);
+                    if (generationTracker != null) {
+                        if (generationTracker.isGenerationChanged()) {
                             if (DEBUG) {
-                                Log.i(TAG, "Cache hit for prefix:" + prefix);
+                                Log.i(TAG, "Generation changed for prefix:" + prefix
+                                        + " type:" + mUri.getPath()
+                                        + " in package:" + cr.getPackageName());
                             }
-                            if (!names.isEmpty()) {
-                                for (String name : names) {
-                                    if (mValues.containsKey(name)) {
-                                        keyValues.put(name, mValues.get(name));
-                                    }
+                            for (int i = mValues.size() - 1; i >= 0; i--) {
+                                String key = mValues.keyAt(i);
+                                if (key.startsWith(prefix)) {
+                                    mValues.remove(key);
+                                }
+                            }
+                        } else {
+                            boolean prefixCached = mValues.containsKey(prefix);
+                            if (prefixCached) {
+                                if (DEBUG) {
+                                    Log.i(TAG, "Cache hit for prefix:" + prefix);
                                 }
-                            } else {
-                                for (int i = 0; i < mValues.size(); ++i) {
-                                    String key = mValues.keyAt(i);
-                                    // Explicitly exclude the prefix as it is only there to
-                                    // signal that the prefix has been cached.
-                                    if (key.startsWith(prefix) && !key.equals(prefix)) {
-                                        keyValues.put(key, mValues.get(key));
+                                if (!names.isEmpty()) {
+                                    for (String name : names) {
+                                        if (mValues.containsKey(name)) {
+                                            keyValues.put(name, mValues.get(name));
+                                        }
+                                    }
+                                } else {
+                                    for (int i = 0; i < mValues.size(); ++i) {
+                                        String key = mValues.keyAt(i);
+                                        // Explicitly exclude the prefix as it is only there to
+                                        // signal that the prefix has been cached.
+                                        if (key.startsWith(prefix) && !key.equals(prefix)) {
+                                            keyValues.put(key, mValues.get(key));
+                                        }
                                     }
                                 }
+                                return keyValues;
                             }
-                            return keyValues;
                         }
+                        currentGeneration = generationTracker.getCurrentGeneration();
+                    } else {
+                        needsGenerationTracker = true;
                     }
-                    currentGeneration = generationTracker.getCurrentGeneration();
-                } else {
-                    needsGenerationTracker = true;
+                }
+            } else {
+                if (DEBUG || LOCAL_LOGV) {
+                    Log.v(TAG, "getting settings for prefix " + prefix + " in system_server"
+                            + " so skipping cache");
                 }
             }