Merge "Retry property set failures" into main

3 files changed, 107 insertions, 46 deletions

diff --git a/core/java/android/app/PropertyInvalidatedCache.java b/core/java/android/app/PropertyInvalidatedCache.java
index 5567c085d205..e7e9b0027812 100644
--- a/core/java/android/app/PropertyInvalidatedCache.java
+++ b/core/java/android/app/PropertyInvalidatedCache.java
@@ -36,6 +36,7 @@ import android.util.ArraySet;
 import android.util.Log;
 import android.util.SparseArray;
 import android.util.SparseBooleanArray;
+import android.util.SystemPropertySetter;
 
 import com.android.internal.annotations.GuardedBy;
 import com.android.internal.annotations.VisibleForTesting;
@@ -157,21 +158,6 @@ public class PropertyInvalidatedCache<Query, Result> {
     public static final String MODULE_TELEPHONY = "telephony";
 
     /**
-     * Constants that affect retries when the process is unable to write the property.
-     * The first constant is the number of times the process will attempt to set the
-     * property.  The second constant is the delay between attempts.
-     */
-
-    /**
-     * Wait 200ms between retry attempts and the retry limit is 5.  That gives a total possible
-     * delay of 1s, which should be less than ANR timeouts.  The goal is to have the system crash
-     * because the property could not be set (which is a condition that is easily recognized) and
-     * not crash because of an ANR (which can be confusing to debug).
-     */
-    private static final int PROPERTY_FAILURE_RETRY_DELAY_MILLIS = 200;
-    private static final int PROPERTY_FAILURE_RETRY_LIMIT = 5;
-
-    /**
      * Construct a system property that matches the rules described above.  The module is
      * one of the permitted values above.  The API is a string that is a legal Java simple
      * identifier.  The api is modified to conform to the system property style guide by
@@ -958,37 +944,8 @@ public class PropertyInvalidatedCache<Query, Result> {
          */
         @Override
         void setNonceInternal(long value) {
-            // Failing to set the nonce is a fatal error.  Failures setting a system property have
-            // been reported; given that the failure is probably transient, this function includes
-            // a retry.
             final String str = Long.toString(value);
-            RuntimeException failure = null;
-            for (int attempt = 0; attempt < PROPERTY_FAILURE_RETRY_LIMIT; attempt++) {
-                try {
-                    SystemProperties.set(mName, str);
-                    if (attempt > 0) {
-                        // This log is not guarded.  Based on known bug reports, it should
-                        // occur once a week or less.  The purpose of the log message is to
-                        // identify the retries as a source of delay that might be otherwise
-                        // be attributed to the cache itself.
-                        Log.w(TAG, "Nonce set after " + attempt + " tries");
-                    }
-                    return;
-                } catch (RuntimeException e) {
-                    if (failure == null) {
-                        failure = e;
-                    }
-                    try {
-                        Thread.sleep(PROPERTY_FAILURE_RETRY_DELAY_MILLIS);
-                    } catch (InterruptedException x) {
-                        // Ignore this exception.  The desired delay is only approximate and
-                        // there is no issue if the sleep sometimes terminates early.
-                    }
-                }
-            }
-            // This point is reached only if SystemProperties.set() fails at least once.
-            // Rethrow the first exception that was received.
-            throw failure;
+            SystemPropertySetter.setWithRetry(mName, str);
         }
     }
 
diff --git a/core/java/android/util/SystemPropertySetter.java b/core/java/android/util/SystemPropertySetter.java
new file mode 100644
index 000000000000..bf18f753231e
--- /dev/null
+++ b/core/java/android/util/SystemPropertySetter.java
@@ -0,0 +1,103 @@
+/*
+ * Copyright (C) 2025 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package android.util;
+
+import android.annotation.NonNull;
+import android.annotation.Nullable;
+import android.os.SystemProperties;
+
+/**
+ * This class provides a single, static function to set a system property.  The function retries on
+ * error.  System properties should be reliable but there have been reports of failures in the set()
+ * command on lower-end devices.  Clients may want to use this method instead of calling
+ * {@link SystemProperties.set} directly.
+ * @hide
+ */
+public class SystemPropertySetter {
+
+    /**
+     * The default retryDelayMs for {@link #setWithRetry}.  This value has been found to give
+     * reasonable behavior in the field.
+     */
+    public static final int PROPERTY_FAILURE_RETRY_DELAY_MILLIS = 200;
+
+    /**
+     * The default retryLimit for {@link #setWithRetry}.  This value has been found to give
+     * reasonable behavior in the field.
+     */
+    public static final int PROPERTY_FAILURE_RETRY_LIMIT = 5;
+
+    /**
+     * Set the value for the given {@code key} to {@code val}.  This method retries using the
+     * standard parameters, above, if the native method throws a RuntimeException.
+     *
+     * @param key The name of the property to be set.
+     * @param val The new string value of the property.
+     * @throws IllegalArgumentException for non read-only properties if the {@code val} exceeds
+     * 91 characters.
+     * @throws RuntimeException if the property cannot be set, for example, if it was blocked by
+     * SELinux. libc will log the underlying reason.
+     */
+    public static void setWithRetry(@NonNull String key, @Nullable String val) {
+        setWithRetry(key, val,PROPERTY_FAILURE_RETRY_DELAY_MILLIS, PROPERTY_FAILURE_RETRY_LIMIT);
+    }
+
+    /**
+     * Set the value for the given {@code key} to {@code val}.  This method retries if the native
+     * method throws a RuntimeException.  If the {@code maxRetry} count is exceeded, the method
+     * throws the first RuntimeException that was seen.
+     *
+     * @param key The name of the property to be set.
+     * @param val The new string value of the property.
+     * @param maxRetry The maximum number of times; must be non-negative.
+     * @param retryDelayMs The number of milliseconds to wait between retries; must be positive.
+     * @throws IllegalArgumentException for non read-only properties if the {@code val} exceeds
+     * 91 characters, or if the retry parameters are invalid.
+     * @throws RuntimeException if the property cannot be set, for example, if it was blocked by
+     * SELinux. libc will log the underlying reason.
+     */
+    public static void setWithRetry(@NonNull String key, @Nullable String val, int maxRetry,
+            long retryDelayMs) {
+        if (maxRetry < 0) {
+            throw new IllegalArgumentException("invalid retry count: " + maxRetry);
+        }
+        if (retryDelayMs <= 0) {
+            throw new IllegalArgumentException("invalid retry delay: " + retryDelayMs);
+        }
+
+        RuntimeException failure = null;
+        for (int attempt = 0; attempt < maxRetry; attempt++) {
+            try {
+                SystemProperties.set(key, val);
+                return;
+            } catch (RuntimeException e) {
+                if (failure == null) {
+                    failure = e;
+                }
+                try {
+                    Thread.sleep(retryDelayMs);
+                } catch (InterruptedException x) {
+                    // Ignore this exception.  The desired delay is only approximate and
+                    // there is no issue if the sleep sometimes terminates early.
+                }
+            }
+        }
+        // This point is reached only if SystemProperties.set() fails at least once.
+        // Rethrow the first exception that was received.
+        throw failure;
+    }
+}
diff --git a/services/core/java/com/android/server/audio/AudioService.java b/services/core/java/com/android/server/audio/AudioService.java
index 0fd47169122b..bd2714211796 100644
--- a/services/core/java/com/android/server/audio/AudioService.java
+++ b/services/core/java/com/android/server/audio/AudioService.java
@@ -242,6 +242,7 @@ import android.util.PrintWriterPrinter;
 import android.util.Slog;
 import android.util.SparseArray;
 import android.util.SparseIntArray;
+import android.util.SystemPropertySetter;
 import android.view.Display;
 import android.view.KeyEvent;
 import android.view.accessibility.AccessibilityManager;
@@ -11032,7 +11033,7 @@ public class AudioService extends IAudioService.Stub
         @GuardedBy("mLock")
         private void updateLocked() {
             String n = Long.toString(mToken++);
-            SystemProperties.set(PermissionManager.CACHE_KEY_PACKAGE_INFO_NOTIFY, n);
+            SystemPropertySetter.setWithRetry(PermissionManager.CACHE_KEY_PACKAGE_INFO_NOTIFY, n);
         }
 
         private void trigger() {