diff options
7 files changed, 170 insertions, 31 deletions
diff --git a/core/api/test-current.txt b/core/api/test-current.txt index 0db91f5667ee..f7ac9459376b 100644 --- a/core/api/test-current.txt +++ b/core/api/test-current.txt @@ -1992,6 +1992,7 @@ package android.media { method @RequiresPermission(anyOf={android.Manifest.permission.MODIFY_AUDIO_ROUTING, android.Manifest.permission.QUERY_AUDIO_STATE, android.Manifest.permission.MODIFY_AUDIO_SETTINGS_PRIVILEGED}) public boolean isFullVolumeDevice(); method @RequiresPermission(android.Manifest.permission.CALL_AUDIO_INTERCEPTION) public boolean isPstnCallAudioInterceptable(); method @RequiresPermission(android.Manifest.permission.MODIFY_AUDIO_SETTINGS_PRIVILEGED) public boolean isVolumeControlUsingVolumeGroups(); + method public void permissionUpdateBarrier(); method @RequiresPermission("android.permission.QUERY_AUDIO_STATE") public int requestAudioFocusForTest(@NonNull android.media.AudioFocusRequest, @NonNull String, int, int); method @RequiresPermission(android.Manifest.permission.MODIFY_AUDIO_SETTINGS_PRIVILEGED) public void setCsd(float); method @RequiresPermission(android.Manifest.permission.MODIFY_AUDIO_SETTINGS_PRIVILEGED) public void setNotifAliasRingForTest(boolean); diff --git a/media/java/android/media/AudioManager.java b/media/java/android/media/AudioManager.java index c36eda908d32..c6305757b594 100644 --- a/media/java/android/media/AudioManager.java +++ b/media/java/android/media/AudioManager.java @@ -10128,6 +10128,24 @@ public class AudioManager { /** * @hide + * Blocks until permission updates have propagated through the audio system. + * Only useful in tests, where adoptShellPermissions can change the permission state of + * an app without the app being killed. + */ + @TestApi + @SuppressWarnings("UnflaggedApi") // @TestApi without associated feature. + public void permissionUpdateBarrier() { + final IAudioService service = getService(); + try { + service.permissionUpdateBarrier(); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); + } + } + + + /** + * @hide * Return the list of independent stream types for volume control. * A stream type is considered independent when the volume changes of that type do not * affect any other independent volume control stream type. diff --git a/media/java/android/media/IAudioService.aidl b/media/java/android/media/IAudioService.aidl index d20b7f090e92..e0c346100d03 100644 --- a/media/java/android/media/IAudioService.aidl +++ b/media/java/android/media/IAudioService.aidl @@ -101,6 +101,8 @@ interface IAudioService { oneway void portEvent(in int portId, in int event, in @nullable PersistableBundle extras); + void permissionUpdateBarrier(); + // Java-only methods below. void adjustStreamVolume(int streamType, int direction, int flags, String callingPackage); diff --git a/media/tests/mediatestutils/Android.bp b/media/tests/mediatestutils/Android.bp index 88938e2f71d0..8c68f210ce3d 100644 --- a/media/tests/mediatestutils/Android.bp +++ b/media/tests/mediatestutils/Android.bp @@ -27,9 +27,11 @@ java_library { name: "mediatestutils", srcs: [ "java/com/android/media/mediatestutils/TestUtils.java", + "java/com/android/media/mediatestutils/PermissionUpdateBarrierRule.java", ], static_libs: [ "androidx.concurrent_concurrent-futures", + "androidx.test.runner", "guava", "mediatestutils_host", ], diff --git a/media/tests/mediatestutils/java/com/android/media/mediatestutils/PermissionUpdateBarrierRule.java b/media/tests/mediatestutils/java/com/android/media/mediatestutils/PermissionUpdateBarrierRule.java new file mode 100644 index 000000000000..c51b5dea9546 --- /dev/null +++ b/media/tests/mediatestutils/java/com/android/media/mediatestutils/PermissionUpdateBarrierRule.java @@ -0,0 +1,59 @@ +/* + * Copyright 2024 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 com.android.media.mediatestutils; + +import android.content.Context; +import android.media.AudioManager; + +import androidx.test.platform.app.InstrumentationRegistry; + +import org.junit.rules.TestRule; +import org.junit.runner.Description; +import org.junit.runners.model.Statement; + +/** + * Barrier to wait for permission updates to propagate to audioserver, to avoid flakiness when using + * {@code com.android.compatability.common.util.AdoptShellPermissionsRule}. Note, this rule should + * <b> always </b> be placed after the adopt permission rule. Don't use rule when changing + * permission state in {@code @Before}, since that executes after all rules. + */ +public class PermissionUpdateBarrierRule implements TestRule { + + private final Context mContext; + + /** + * @param context the context to use + */ + public PermissionUpdateBarrierRule(Context context) { + mContext = context; + } + + public PermissionUpdateBarrierRule() { + this(InstrumentationRegistry.getInstrumentation().getContext()); + } + + @Override + public Statement apply(Statement base, Description description) { + return new Statement() { + @Override + public void evaluate() throws Throwable { + mContext.getSystemService(AudioManager.class).permissionUpdateBarrier(); + base.evaluate(); + } + }; + } +} diff --git a/services/core/java/com/android/server/audio/AudioServerPermissionProvider.java b/services/core/java/com/android/server/audio/AudioServerPermissionProvider.java index c5180afcce7d..5283eddd90fb 100644 --- a/services/core/java/com/android/server/audio/AudioServerPermissionProvider.java +++ b/services/core/java/com/android/server/audio/AudioServerPermissionProvider.java @@ -33,6 +33,7 @@ import static android.Manifest.permission.WRITE_SECURE_SETTINGS; import android.annotation.Nullable; import android.os.RemoteException; +import android.os.Trace; import android.os.UserHandle; import android.util.ArraySet; import android.util.IntArray; @@ -190,6 +191,7 @@ public class AudioServerPermissionProvider { mIsUpdateDeferred = true; return; } + Trace.traceBegin(Trace.TRACE_TAG_SYSTEM_SERVER, "audioserver_permission_update"); try { for (byte i = 0; i < PermissionEnum.ENUM_SIZE; i++) { var newPerms = getUidsHoldingPerm(i); @@ -203,6 +205,8 @@ public class AudioServerPermissionProvider { mDest = null; // We didn't necessarily finish mIsUpdateDeferred = true; + } finally { + Trace.traceEnd(Trace.TRACE_TAG_SYSTEM_SERVER); } } } diff --git a/services/core/java/com/android/server/audio/AudioService.java b/services/core/java/com/android/server/audio/AudioService.java index 53b04df4652e..930fb397a5a2 100644 --- a/services/core/java/com/android/server/audio/AudioService.java +++ b/services/core/java/com/android/server/audio/AudioService.java @@ -284,11 +284,16 @@ import java.util.NoSuchElementException; import java.util.Objects; import java.util.Set; import java.util.TreeSet; +import java.util.concurrent.CancellationException; +import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.concurrent.ExecutionException; import java.util.concurrent.Executor; import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; -import java.util.concurrent.atomic.AtomicLong; import java.util.function.BooleanSupplier; import java.util.stream.Collectors; @@ -785,6 +790,8 @@ public class AudioService extends IAudioService.Stub private final BroadcastReceiver mReceiver = new AudioServiceBroadcastReceiver(); private final Executor mAudioServerLifecycleExecutor; + private final ConcurrentLinkedQueue<Future> mScheduledPermissionTasks = + new ConcurrentLinkedQueue(); private IMediaProjectionManager mProjectionService; // to validate projection token @@ -1092,7 +1099,8 @@ public class AudioService extends IAudioService.Stub public Lifecycle(Context context) { super(context); - var audioserverLifecycleExecutor = Executors.newSingleThreadExecutor(); + var audioserverLifecycleExecutor = Executors.newSingleThreadScheduledExecutor( + (Runnable r) -> new Thread(r, "audioserver_lifecycle")); var audioPolicyFacade = new DefaultAudioPolicyFacade(audioserverLifecycleExecutor); mService = new AudioService(context, AudioSystemAdapter.getDefaultAdapter(), @@ -1222,34 +1230,6 @@ 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); @@ -1717,8 +1697,10 @@ public class AudioService extends IAudioService.Stub public void onSystemReady() { mSystemReady = true; + if (audioserverPermissions()) { + setupPermissionListener(); + } scheduleLoadSoundEffects(); - mDeviceBroker.onSystemReady(); if (mContext.getPackageManager().hasSystemFeature(PackageManager.FEATURE_HDMI_CEC)) { @@ -10608,6 +10590,63 @@ public class AudioService extends IAudioService.Stub } } + /* Listen to permission invalidations for the PermissionProvider */ + private void setupPermissionListener() { + // Roughly chosen to be long enough to suppress the autocork behavior of the permission + // cache (50ms), 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 = 60; + // instanceof to simplify the construction requirements of AudioService for testing: no + // delayed execution during unit tests. + if (mAudioServerLifecycleExecutor instanceof ScheduledExecutorService exec) { + // We schedule and add from a this callback thread only (serially), so the task order on + // the serial executor matches the order on the task list. This list should almost + // always have only two elements, except in cases of serious system contention. + Runnable task = () -> mScheduledPermissionTasks.add(exec.schedule(() -> { + try { + // Clean up completed tasks before us to bound the queue length. Cancel any + // pending permission refresh tasks, after our own, since we are about to + // fulfill all of them. We must be the first non-completed task in the + // queue, since the execution order matches the queue order. Note, this + // task is the only writer on elements in the queue, and the task is + // serialized, so + // => no in-flight cancellation + // => exists at least one non-completed task (ourselves) + // => the queue is non-empty (only completed tasks removed) + final var iter = mScheduledPermissionTasks.iterator(); + while (iter.next().isDone()) { + iter.remove(); + } + // iter is on the first element which is not completed (us) + while (iter.hasNext()) { + if (!iter.next().cancel(false)) { + throw new AssertionError( + "Cancel should be infallible since we" + + "cancel from the executor"); + } + iter.remove(); + } + mPermissionProvider.onPermissionStateChanged(); + } catch (Exception e) { + // Handle executor routing exceptions to nowhere + Thread.getDefaultUncaughtExceptionHandler() + .uncaughtException(Thread.currentThread(), e); + } + }, + UPDATE_DELAY_MS, + TimeUnit.MILLISECONDS)); + mAudioSystem.listenForSystemPropertyChange( + PermissionManager.CACHE_KEY_PACKAGE_INFO, + task); + task.run(); + } else { + mAudioSystem.listenForSystemPropertyChange( + PermissionManager.CACHE_KEY_PACKAGE_INFO, + () -> mAudioServerLifecycleExecutor.execute( + mPermissionProvider::onPermissionStateChanged)); + } + } //========================================================================================== // Audio Focus @@ -14663,6 +14702,20 @@ public class AudioService extends IAudioService.Stub return activeAssistantUids; } + @Override + /** @see AudioManager#permissionUpdateBarrier() */ + public void permissionUpdateBarrier() { + for (var x : List.copyOf(mScheduledPermissionTasks)) { + try { + x.get(); + } catch (CancellationException e) { + // Task completed + } catch (InterruptedException | ExecutionException e) { + Log.wtf(TAG, "Exception which should never occur", e); + } + } + } + List<String> getDeviceIdentityAddresses(AudioDeviceAttributes device) { return mDeviceBroker.getDeviceIdentityAddresses(device); } |