Migrate audio permission provider prop listener

The java system property listener is non-functional. Migrate the native
permission provider to use a native system property change listener for
cache invalidation.

Add basic throttling to pushing updates following permission cache
invalidation, which delays the update after the sysprop changes and
conflates subsequent updates prior to the delay expiring.

Bug: 338089555
Test: Manual verification of listener responding to sysprop changes
Flag: com.android.media.audio.audioserver_permissions
Change-Id: Ib21120da5c01867ab9ee1ed019c912b307fda7da

9 files changed, 124 insertions, 31 deletions

diff --git a/core/jni/android_media_AudioSystem.cpp b/core/jni/android_media_AudioSystem.cpp
index eaff7608ce3b..a278fea3b9bc 100644
--- a/core/jni/android_media_AudioSystem.cpp
+++ b/core/jni/android_media_AudioSystem.cpp
@@ -27,6 +27,7 @@
 #include <android_media_audiopolicy.h>
 #include <android_os_Parcel.h>
 #include <audiomanager/AudioManager.h>
+#include <android-base/properties.h>
 #include <binder/IBinder.h>
 #include <jni.h>
 #include <media/AidlConversion.h>
@@ -41,8 +42,10 @@
 #include <system/audio_policy.h>
 #include <utils/Log.h>
 
+#include <thread>
 #include <optional>
 #include <sstream>
+#include <memory>
 #include <vector>
 
 #include "android_media_AudioAttributes.h"
@@ -261,6 +264,13 @@ static struct {
     jfieldID mMixerBehavior;
 } gAudioMixerAttributesField;
 
+static struct {
+    jclass clazz;
+    jmethodID run;
+} gRunnableClassInfo;
+
+static JavaVM* gVm;
+
 static Mutex gLock;
 
 enum AudioError {
@@ -3363,6 +3373,55 @@ static jboolean android_media_AudioSystem_isBluetoothVariableLatencyEnabled(JNIE
     return enabled;
 }
 
+class JavaSystemPropertyListener {
+  public:
+    JavaSystemPropertyListener(JNIEnv* env, jobject javaCallback, std::string sysPropName) :
+            mCallback(env->NewGlobalRef(javaCallback)),
+            mCachedProperty(android::base::CachedProperty{std::move(sysPropName)}) {
+        mListenerThread = std::thread([this]() mutable {
+            JNIEnv* threadEnv = GetOrAttachJNIEnvironment(gVm);
+            while (!mCleanupSignal.load()) {
+                using namespace std::chrono_literals;
+                // 1s timeout so this thread can read the cleanup signal to (slowly) be able to
+                // be destroyed.
+                std::string newVal = mCachedProperty.WaitForChange(1000ms) ?: "";
+                if (newVal != "" && mLastVal != newVal) {
+                    threadEnv->CallVoidMethod(mCallback, gRunnableClassInfo.run);
+                    mLastVal = std::move(newVal);
+                }
+            }
+            });
+    }
+
+    ~JavaSystemPropertyListener() {
+        mCleanupSignal.store(true);
+        mListenerThread.join();
+        JNIEnv* env = GetOrAttachJNIEnvironment(gVm);
+        env->DeleteGlobalRef(mCallback);
+    }
+
+  private:
+    jobject mCallback;
+    android::base::CachedProperty mCachedProperty;
+    std::thread mListenerThread;
+    std::atomic<bool> mCleanupSignal{false};
+    std::string mLastVal = "";
+};
+
+std::vector<std::unique_ptr<JavaSystemPropertyListener>> gSystemPropertyListeners;
+std::mutex gSysPropLock{};
+
+static void android_media_AudioSystem_listenForSystemPropertyChange(JNIEnv *env,  jobject thiz,
+        jstring sysProp,
+        jobject javaCallback) {
+    ScopedUtfChars sysPropChars{env, sysProp};
+    auto listener = std::make_unique<JavaSystemPropertyListener>(env, javaCallback,
+            std::string{sysPropChars.c_str()});
+    std::unique_lock _l{gSysPropLock};
+    gSystemPropertyListeners.push_back(std::move(listener));
+}
+
+
 // ----------------------------------------------------------------------------
 
 #define MAKE_AUDIO_SYSTEM_METHOD(x) \
@@ -3535,7 +3594,12 @@ static const JNINativeMethod gMethods[] =
                                 android_media_AudioSystem_clearPreferredMixerAttributes),
          MAKE_AUDIO_SYSTEM_METHOD(supportsBluetoothVariableLatency),
          MAKE_AUDIO_SYSTEM_METHOD(setBluetoothVariableLatencyEnabled),
-         MAKE_AUDIO_SYSTEM_METHOD(isBluetoothVariableLatencyEnabled)};
+         MAKE_AUDIO_SYSTEM_METHOD(isBluetoothVariableLatencyEnabled),
+         MAKE_JNI_NATIVE_METHOD("listenForSystemPropertyChange",
+                                "(Ljava/lang/String;Ljava/lang/Runnable;)V",
+                                android_media_AudioSystem_listenForSystemPropertyChange),
+
+        };
 
 static const JNINativeMethod gEventHandlerMethods[] =
         {MAKE_JNI_NATIVE_METHOD("native_setup", "(Ljava/lang/Object;)V",
@@ -3817,6 +3881,12 @@ int register_android_media_AudioSystem(JNIEnv *env)
     gAudioMixerAttributesField.mMixerBehavior =
             GetFieldIDOrDie(env, audioMixerAttributesClass, "mMixerBehavior", "I");
 
+    jclass runnableClazz = FindClassOrDie(env, "java/lang/Runnable");
+    gRunnableClassInfo.clazz = MakeGlobalRefOrDie(env, runnableClazz);
+    gRunnableClassInfo.run = GetMethodIDOrDie(env, runnableClazz, "run", "()V");
+
+    LOG_ALWAYS_FATAL_IF(env->GetJavaVM(&gVm) != 0);
+
     AudioSystem::addErrorCallback(android_media_AudioSystem_error_callback);
 
     RegisterMethodsOrDie(env, kClassPathName, gMethods, NELEM(gMethods));
diff --git a/media/java/android/media/AudioSystem.java b/media/java/android/media/AudioSystem.java
index d148afd3c15d..52b5ff77c031 100644
--- a/media/java/android/media/AudioSystem.java
+++ b/media/java/android/media/AudioSystem.java
@@ -2649,4 +2649,11 @@ public class AudioSystem
      * @hide
      */
     public static native boolean isBluetoothVariableLatencyEnabled();
+
+    /**
+     * Register a native listener for system property sysprop
+     * @param callback the listener which fires when the property changes
+     * @hide
+     */
+    public static native void listenForSystemPropertyChange(String sysprop, Runnable callback);
 }
diff --git a/services/core/java/com/android/server/audio/AudioService.java b/services/core/java/com/android/server/audio/AudioService.java
index 74d218cba7ab..41b984b67d02 100644
--- a/services/core/java/com/android/server/audio/AudioService.java
+++ b/services/core/java/com/android/server/audio/AudioService.java
@@ -734,6 +734,8 @@ public class AudioService extends IAudioService.Stub
     // Broadcast receiver for device connections intent broadcasts
     private final BroadcastReceiver mReceiver = new AudioServiceBroadcastReceiver();
 
+    private final Executor mAudioServerLifecycleExecutor;
+
     private IMediaProjectionManager mProjectionService; // to validate projection token
 
     /** Interface for UserManagerService. */
@@ -1052,7 +1054,8 @@ public class AudioService extends IAudioService.Stub
                               audioserverPermissions() ?
                                 initializeAudioServerPermissionProvider(
                                     context, audioPolicyFacade, audioserverLifecycleExecutor) :
-                                    null
+                                    null,
+                              audioserverLifecycleExecutor
                               );
         }
 
@@ -1138,13 +1141,16 @@ public class AudioService extends IAudioService.Stub
      *               {@link AudioSystemThread} is created as the messaging thread instead.
      * @param appOps {@link AppOpsManager} system service
      * @param enforcer Used for permission enforcing
+     * @param permissionProvider Used to push permissions to audioserver
+     * @param audioserverLifecycleExecutor Used for tasks managing audioserver lifecycle
      */
     @RequiresPermission(Manifest.permission.READ_DEVICE_CONFIG)
     public AudioService(Context context, AudioSystemAdapter audioSystem,
             SystemServerAdapter systemServer, SettingsAdapter settings,
             AudioVolumeGroupHelperBase audioVolumeGroupHelper, AudioPolicyFacade audioPolicy,
             @Nullable Looper looper, AppOpsManager appOps, @NonNull PermissionEnforcer enforcer,
-            /* @NonNull */ AudioServerPermissionProvider permissionProvider) {
+            /* @NonNull */ AudioServerPermissionProvider permissionProvider,
+            Executor audioserverLifecycleExecutor) {
         super(enforcer);
         sLifecycleLogger.enqueue(new EventLogger.StringEvent("AudioService()"));
         mContext = context;
@@ -1152,6 +1158,7 @@ public class AudioService extends IAudioService.Stub
         mAppOps = appOps;
 
         mPermissionProvider = permissionProvider;
+        mAudioServerLifecycleExecutor = audioserverLifecycleExecutor;
 
         mAudioSystem = audioSystem;
         mSystemServer = systemServer;
@@ -1163,6 +1170,34 @@ public class AudioService extends IAudioService.Stub
         mBroadcastHandlerThread = new HandlerThread("AudioService Broadcast");
         mBroadcastHandlerThread.start();
 
+        // Listen to permission invalidations for the PermissionProvider
+        if (audioserverPermissions()) {
+            final Handler broadcastHandler = mBroadcastHandlerThread.getThreadHandler();
+            mAudioSystem.listenForSystemPropertyChange(PermissionManager.CACHE_KEY_PACKAGE_INFO,
+                    new Runnable() {
+                        // Roughly chosen to be long enough to suppress the autocork behavior
+                        // of the permission cache (50ms), and longer than the task could reasonably
+                        // take, even with many packages and users, while not introducing visible
+                        // permission leaks - since the app needs to restart, and trigger an action
+                        // which requires permissions from audioserver before this delay.
+                        // For RECORD_AUDIO, we are additionally protected by appops.
+                        final long UPDATE_DELAY_MS = 110;
+                        final AtomicLong scheduledUpdateTimestamp = new AtomicLong(0);
+                        @Override
+                        public void run() {
+                            var currentTime = SystemClock.uptimeMillis();
+                            if (currentTime > scheduledUpdateTimestamp.get()) {
+                                scheduledUpdateTimestamp.set(currentTime + UPDATE_DELAY_MS);
+                                broadcastHandler.postAtTime( () ->
+                                        mAudioServerLifecycleExecutor.execute(mPermissionProvider
+                                            ::onPermissionStateChanged),
+                                        currentTime + UPDATE_DELAY_MS
+                                    );
+                            }
+                        }
+            });
+        }
+
         mDeviceBroker = new AudioDeviceBroker(mContext, this, mAudioSystem);
 
         mIsSingleVolume = AudioSystem.isSingleVolume(context);
@@ -11972,29 +12007,6 @@ public class AudioService extends IAudioService.Stub
             provider.onServiceStart(audioPolicy.getPermissionController());
         });
 
-        // Set up event listeners
-        // Must be kept in sync with PermissionManager
-        Runnable cacheSysPropHandler = new Runnable() {
-            private AtomicReference<SystemProperties.Handle> mHandle = new AtomicReference();
-            private AtomicLong mNonce = new AtomicLong();
-            @Override
-            public void run() {
-                if (mHandle.get() == null) {
-                    // Cache the handle
-                    mHandle.compareAndSet(null, SystemProperties.find(
-                            PermissionManager.CACHE_KEY_PACKAGE_INFO));
-                }
-                long nonce;
-                SystemProperties.Handle ref;
-                if ((ref = mHandle.get()) != null && (nonce = ref.getLong(0)) != 0 &&
-                        mNonce.getAndSet(nonce) != nonce) {
-                    audioserverExecutor.execute(() -> provider.onPermissionStateChanged());
-                }
-            }
-        };
-
-        SystemProperties.addChangeCallback(cacheSysPropHandler);
-
         IntentFilter packageUpdateFilter = new IntentFilter();
         packageUpdateFilter.addAction(ACTION_PACKAGE_ADDED);
         packageUpdateFilter.addAction(ACTION_PACKAGE_REMOVED);
diff --git a/services/core/java/com/android/server/audio/AudioSystemAdapter.java b/services/core/java/com/android/server/audio/AudioSystemAdapter.java
index 7f4bc74bd59e..d083c68c4c2c 100644
--- a/services/core/java/com/android/server/audio/AudioSystemAdapter.java
+++ b/services/core/java/com/android/server/audio/AudioSystemAdapter.java
@@ -748,6 +748,10 @@ public class AudioSystemAdapter implements AudioSystem.RoutingUpdateCallback,
         return AudioSystem.setMasterMute(mute);
     }
 
+    public void listenForSystemPropertyChange(String systemPropertyName, Runnable callback) {
+        AudioSystem.listenForSystemPropertyChange(systemPropertyName, callback);
+    }
+
     /**
      * Part of AudioService dump
      * @param pw
diff --git a/services/tests/servicestests/src/com/android/server/audio/AbsoluteVolumeBehaviorTest.java b/services/tests/servicestests/src/com/android/server/audio/AbsoluteVolumeBehaviorTest.java
index 758c84a26dcd..ef9580c54de6 100644
--- a/services/tests/servicestests/src/com/android/server/audio/AbsoluteVolumeBehaviorTest.java
+++ b/services/tests/servicestests/src/com/android/server/audio/AbsoluteVolumeBehaviorTest.java
@@ -101,7 +101,7 @@ public class AbsoluteVolumeBehaviorTest {
         mAudioService = new AudioService(mContext, mSpyAudioSystem, mSystemServer,
                 mSettingsAdapter, mAudioVolumeGroupHelper, mMockAudioPolicy,
                 mTestLooper.getLooper(), mock(AppOpsManager.class), mock(PermissionEnforcer.class),
-                mock(AudioServerPermissionProvider.class)) {
+                mock(AudioServerPermissionProvider.class), r -> r.run()) {
             @Override
             public int getDeviceForStream(int stream) {
                 return AudioSystem.DEVICE_OUT_SPEAKER;
diff --git a/services/tests/servicestests/src/com/android/server/audio/AudioDeviceVolumeManagerTest.java b/services/tests/servicestests/src/com/android/server/audio/AudioDeviceVolumeManagerTest.java
index 2cb02bdd2806..464515632997 100644
--- a/services/tests/servicestests/src/com/android/server/audio/AudioDeviceVolumeManagerTest.java
+++ b/services/tests/servicestests/src/com/android/server/audio/AudioDeviceVolumeManagerTest.java
@@ -78,7 +78,7 @@ public class AudioDeviceVolumeManagerTest {
         mAudioService = new AudioService(mContext, mSpyAudioSystem, mSystemServer,
                 mSettingsAdapter, mAudioVolumeGroupHelper, mAudioPolicyMock,
                 mTestLooper.getLooper(), mock(AppOpsManager.class), mock(PermissionEnforcer.class),
-                mock(AudioServerPermissionProvider.class)) {
+                mock(AudioServerPermissionProvider.class), r -> r.run()) {
             @Override
             public int getDeviceForStream(int stream) {
                 return AudioSystem.DEVICE_OUT_SPEAKER;
diff --git a/services/tests/servicestests/src/com/android/server/audio/AudioServiceTest.java b/services/tests/servicestests/src/com/android/server/audio/AudioServiceTest.java
index 037c3c00443c..b7100ea00a40 100644
--- a/services/tests/servicestests/src/com/android/server/audio/AudioServiceTest.java
+++ b/services/tests/servicestests/src/com/android/server/audio/AudioServiceTest.java
@@ -87,7 +87,7 @@ public class AudioServiceTest {
                 .thenReturn(AppOpsManager.MODE_ALLOWED);
         mAudioService = new AudioService(mContext, mSpyAudioSystem, mSpySystemServer,
                 mSettingsAdapter, mAudioVolumeGroupHelper, mMockAudioPolicy, null,
-                mMockAppOpsManager, mMockPermissionEnforcer, mMockPermissionProvider);
+                mMockAppOpsManager, mMockPermissionEnforcer, mMockPermissionProvider, r -> r.run());
     }
 
     /**
diff --git a/services/tests/servicestests/src/com/android/server/audio/DeviceVolumeBehaviorTest.java b/services/tests/servicestests/src/com/android/server/audio/DeviceVolumeBehaviorTest.java
index 27b552fa7cdd..746645a8c585 100644
--- a/services/tests/servicestests/src/com/android/server/audio/DeviceVolumeBehaviorTest.java
+++ b/services/tests/servicestests/src/com/android/server/audio/DeviceVolumeBehaviorTest.java
@@ -78,7 +78,7 @@ public class DeviceVolumeBehaviorTest {
         mAudioService = new AudioService(mContext, mAudioSystem, mSystemServer,
                 mSettingsAdapter, mAudioVolumeGroupHelper, mAudioPolicyMock,
                 mTestLooper.getLooper(), mock(AppOpsManager.class), mock(PermissionEnforcer.class),
-                mock(AudioServerPermissionProvider.class));
+                mock(AudioServerPermissionProvider.class), r -> r.run());
         mTestLooper.dispatchAll();
     }
 
diff --git a/services/tests/servicestests/src/com/android/server/audio/VolumeHelperTest.java b/services/tests/servicestests/src/com/android/server/audio/VolumeHelperTest.java
index 8e34ee1b6a42..e45ab319146c 100644
--- a/services/tests/servicestests/src/com/android/server/audio/VolumeHelperTest.java
+++ b/services/tests/servicestests/src/com/android/server/audio/VolumeHelperTest.java
@@ -160,7 +160,7 @@ public class VolumeHelperTest {
                 @NonNull PermissionEnforcer enforcer,
                 AudioServerPermissionProvider permissionProvider) {
             super(context, audioSystem, systemServer, settings, audioVolumeGroupHelper,
-                    audioPolicy, looper, appOps, enforcer, permissionProvider);
+                    audioPolicy, looper, appOps, enforcer, permissionProvider, r -> r.run());
         }
 
         public void setDeviceForStream(int stream, int device) {