Add ServiceHolder for AudioPolicy
Subsequent commits require registering tasks on the audioserver
lifecycle.
Pull out management of the binder service lifecycle into a holder class
which can register tasks on start/death. Modify DefaultAudioPolicyFacade
to use the new ServiceHolder.
Bug: 338089555
Test: atest ServiceHolderTest
Test: atest AudioManagerTest before and after kill audioserver
Flag: EXEMPT: mechanical refactoring
Change-Id: I2a72b0800d5073d3007bf46561ffc857fc43fe47
diff --git a/services/core/java/com/android/server/audio/AudioService.java b/services/core/java/com/android/server/audio/AudioService.java
index add8491..10a7309 100644
--- a/services/core/java/com/android/server/audio/AudioService.java
+++ b/services/core/java/com/android/server/audio/AudioService.java
@@ -1000,7 +1000,7 @@
SystemServerAdapter.getDefaultAdapter(context),
SettingsAdapter.getDefaultAdapter(),
new AudioVolumeGroupHelper(),
- new DefaultAudioPolicyFacade(),
+ new DefaultAudioPolicyFacade(r -> r.run()),
null);
}
diff --git a/services/core/java/com/android/server/audio/DefaultAudioPolicyFacade.java b/services/core/java/com/android/server/audio/DefaultAudioPolicyFacade.java
index 37b8126..75febbc 100644
--- a/services/core/java/com/android/server/audio/DefaultAudioPolicyFacade.java
+++ b/services/core/java/com/android/server/audio/DefaultAudioPolicyFacade.java
@@ -16,100 +16,43 @@
package com.android.server.audio;
-import android.annotation.NonNull;
-import android.annotation.Nullable;
import android.media.IAudioPolicyService;
-import android.media.permission.ClearCallingIdentityContext;
-import android.media.permission.SafeCloseable;
import android.os.IBinder;
import android.os.RemoteException;
-import android.os.ServiceManager;
-import android.util.Log;
-import com.android.internal.annotations.GuardedBy;
+import java.util.concurrent.Executor;
+import java.util.function.Function;
/**
- * Default implementation of a facade to IAudioPolicyManager which fulfills AudioService
- * dependencies. This forwards calls as-is to IAudioPolicyManager.
- * Public methods throw IllegalStateException if AudioPolicy is not initialized/available
+ * Default implementation of a facade to IAudioPolicyService which fulfills AudioService
+ * dependencies. This forwards calls as-is to IAudioPolicyService.
*/
-public class DefaultAudioPolicyFacade implements AudioPolicyFacade, IBinder.DeathRecipient {
+public class DefaultAudioPolicyFacade implements AudioPolicyFacade {
- private static final String TAG = "DefaultAudioPolicyFacade";
private static final String AUDIO_POLICY_SERVICE_NAME = "media.audio_policy";
- private final Object mServiceLock = new Object();
- @GuardedBy("mServiceLock")
- private IAudioPolicyService mAudioPolicy;
+ private final ServiceHolder<IAudioPolicyService> mServiceHolder;
- public DefaultAudioPolicyFacade() {
- try {
- getAudioPolicyOrInit();
- } catch (IllegalStateException e) {
- // Log and suppress this exception, we may be able to connect later
- Log.e(TAG, "Failed to initialize APM connection", e);
- }
+ /**
+ * @param e - Executor for service start tasks
+ */
+ public DefaultAudioPolicyFacade(Executor e) {
+ mServiceHolder =
+ new ServiceHolder(
+ AUDIO_POLICY_SERVICE_NAME,
+ (Function<IBinder, IAudioPolicyService>)
+ IAudioPolicyService.Stub::asInterface,
+ e);
}
@Override
public boolean isHotwordStreamSupported(boolean lookbackAudio) {
- IAudioPolicyService ap = getAudioPolicyOrInit();
- try (SafeCloseable ignored = ClearCallingIdentityContext.create()) {
+ IAudioPolicyService ap = mServiceHolder.waitForService();
+ try {
return ap.isHotwordStreamSupported(lookbackAudio);
} catch (RemoteException e) {
- resetServiceConnection(ap.asBinder());
- throw new IllegalStateException(e);
- }
- }
-
- @Override
- public void binderDied() {
- Log.wtf(TAG, "Unexpected binderDied without IBinder object");
- }
-
- @Override
- public void binderDied(@NonNull IBinder who) {
- resetServiceConnection(who);
- }
-
- private void resetServiceConnection(@Nullable IBinder deadAudioPolicy) {
- synchronized (mServiceLock) {
- if (mAudioPolicy != null && mAudioPolicy.asBinder().equals(deadAudioPolicy)) {
- mAudioPolicy.asBinder().unlinkToDeath(this, 0);
- mAudioPolicy = null;
- }
- }
- }
-
- private @Nullable IAudioPolicyService getAudioPolicy() {
- synchronized (mServiceLock) {
- return mAudioPolicy;
- }
- }
-
- /*
- * Does not block.
- * @throws IllegalStateException for any failed connection
- */
- private @NonNull IAudioPolicyService getAudioPolicyOrInit() {
- synchronized (mServiceLock) {
- if (mAudioPolicy != null) {
- return mAudioPolicy;
- }
- // Do not block while attempting to connect to APM. Defer to caller.
- IAudioPolicyService ap = IAudioPolicyService.Stub.asInterface(
- ServiceManager.checkService(AUDIO_POLICY_SERVICE_NAME));
- if (ap == null) {
- throw new IllegalStateException(TAG + ": Unable to connect to AudioPolicy");
- }
- try {
- ap.asBinder().linkToDeath(this, 0);
- } catch (RemoteException e) {
- throw new IllegalStateException(
- TAG + ": Unable to link deathListener to AudioPolicy", e);
- }
- mAudioPolicy = ap;
- return mAudioPolicy;
+ mServiceHolder.attemptClear(ap.asBinder());
+ throw new IllegalStateException();
}
}
}
diff --git a/services/core/java/com/android/server/audio/ServiceHolder.java b/services/core/java/com/android/server/audio/ServiceHolder.java
new file mode 100644
index 0000000..e2588fb
--- /dev/null
+++ b/services/core/java/com/android/server/audio/ServiceHolder.java
@@ -0,0 +1,219 @@
+/*
+ * 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.server.audio;
+
+import android.annotation.NonNull;
+import android.annotation.Nullable;
+import android.os.IBinder;
+import android.os.IInterface;
+import android.os.IServiceCallback;
+import android.os.RemoteException;
+import android.os.ServiceManager;
+import android.util.Log;
+
+import java.util.Objects;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.Executor;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Consumer;
+import java.util.function.Function;
+
+/**
+ * Manages a remote service which can start and stop. Allows clients to add tasks to run when the
+ * remote service starts or dies.
+ *
+ * <p>Example usage should look something like:
+ *
+ * <pre>
+ * var service = mServiceHolder.checkService();
+ * if (service == null) handleFailure();
+ * try {
+ * service.foo();
+ * } catch (RemoteException e) {
+ * mServiceHolder.attemptClear(service.asBinder());
+ * handleFailure();
+ * }
+ * </pre>
+ */
+public class ServiceHolder<I extends IInterface> implements IBinder.DeathRecipient {
+
+ private final String mTag;
+ private final String mServiceName;
+ private final Function<? super IBinder, ? extends I> mCastFunction;
+ private final Executor mExecutor;
+ private final ServiceProviderFacade mServiceProvider;
+
+ private final AtomicReference<I> mService = new AtomicReference();
+ private final Set<Consumer<I>> mOnStartTasks = ConcurrentHashMap.newKeySet();
+ private final Set<Consumer<I>> mOnDeathTasks = ConcurrentHashMap.newKeySet();
+
+ private final IServiceCallback mServiceListener =
+ new IServiceCallback.Stub() {
+ @Override
+ public void onRegistration(String name, IBinder binder) {
+ onServiceInited(binder);
+ }
+ };
+
+ // For test purposes
+ public static interface ServiceProviderFacade {
+ public void registerForNotifications(String name, IServiceCallback listener);
+
+ public IBinder checkService(String name);
+
+ public IBinder waitForService(String name);
+ }
+
+ public ServiceHolder(
+ @NonNull String serviceName,
+ @NonNull Function<? super IBinder, ? extends I> castFunction,
+ @NonNull Executor executor) {
+ this(
+ serviceName,
+ castFunction,
+ executor,
+ new ServiceProviderFacade() {
+ @Override
+ public void registerForNotifications(String name, IServiceCallback listener) {
+ try {
+ ServiceManager.registerForNotifications(name, listener);
+ } catch (RemoteException e) {
+ throw new IllegalStateException("ServiceManager died!!", e);
+ }
+ }
+
+ @Override
+ public IBinder checkService(String name) {
+ return ServiceManager.checkService(name);
+ }
+
+ @Override
+ public IBinder waitForService(String name) {
+ return ServiceManager.waitForService(name);
+ }
+ });
+ }
+
+ public ServiceHolder(
+ @NonNull String serviceName,
+ @NonNull Function<? super IBinder, ? extends I> castFunction,
+ @NonNull Executor executor,
+ @NonNull ServiceProviderFacade provider) {
+ mServiceName = Objects.requireNonNull(serviceName);
+ mCastFunction = Objects.requireNonNull(castFunction);
+ mExecutor = Objects.requireNonNull(executor);
+ mServiceProvider = Objects.requireNonNull(provider);
+ mTag = "ServiceHolder: " + serviceName;
+ mServiceProvider.registerForNotifications(mServiceName, mServiceListener);
+ }
+
+ /**
+ * Add tasks to run when service becomes available. Ran on the executor provided at
+ * construction. Note, for convenience, if the service is already connected, the task is
+ * immediately run.
+ */
+ public void registerOnStartTask(Consumer<I> task) {
+ mOnStartTasks.add(task);
+ I i;
+ if ((i = mService.get()) != null) {
+ mExecutor.execute(() -> task.accept(i));
+ }
+ }
+
+ public void unregisterOnStartTask(Consumer<I> task) {
+ mOnStartTasks.remove(task);
+ }
+
+ /**
+ * Add tasks to run when service goes down. Ran on the executor provided at construction. Should
+ * be called before getService to avoid dropping a death notification.
+ */
+ public void registerOnDeathTask(Consumer<I> task) {
+ mOnDeathTasks.add(task);
+ }
+
+ public void unregisterOnDeathTask(Consumer<I> task) {
+ mOnDeathTasks.remove(task);
+ }
+
+ @Override
+ public void binderDied(@NonNull IBinder who) {
+ attemptClear(who);
+ }
+
+ @Override
+ public void binderDied() {
+ throw new AssertionError("Wrong binderDied called, this should never happen");
+ }
+
+ /**
+ * Notify the holder that the service has gone done, usually in response to a RemoteException.
+ * Equivalent to receiving a binder death notification.
+ */
+ public void attemptClear(IBinder who) {
+ // Possibly prone to weird races, resulting in spurious dead/revive,
+ // but that should be fine.
+ var current = mService.get();
+ if (current != null
+ && Objects.equals(current.asBinder(), who)
+ && mService.compareAndSet(current, null)) {
+ who.unlinkToDeath(this, 0);
+ for (var r : mOnDeathTasks) {
+ mExecutor.execute(() -> r.accept(current));
+ }
+ }
+ }
+
+ /** Get the service, without blocking. Can trigger start tasks, on the provided executor. */
+ public @Nullable I checkService() {
+ var s = mService.get();
+ if (s != null) return s;
+ IBinder registered = mServiceProvider.checkService(mServiceName);
+ if (registered == null) return null;
+ return onServiceInited(registered);
+ }
+
+ /** Get the service, but block. Can trigger start tasks, on the provided executor. */
+ public @NonNull I waitForService() {
+ var s = mService.get();
+ return (s != null) ? s : onServiceInited(mServiceProvider.waitForService(mServiceName));
+ }
+
+ /*
+ * Called when the native service is initialized.
+ */
+ private @NonNull I onServiceInited(@NonNull IBinder who) {
+ var service = mCastFunction.apply(who);
+ Objects.requireNonNull(service);
+ if (!mService.compareAndSet(null, service)) {
+ return service;
+ }
+ // Even if the service has immediately died, we should perform these tasks for consistency
+ for (var r : mOnStartTasks) {
+ mExecutor.execute(() -> r.accept(service));
+ }
+ try {
+ who.linkToDeath(this, 0);
+ } catch (RemoteException e) {
+ Log.e(mTag, "Immediate service death. Service crash-looping");
+ attemptClear(who);
+ }
+ // This interface is non-null, but could represent a dead object
+ return service;
+ }
+}
diff --git a/services/tests/servicestests/src/com/android/server/audio/ServiceHolderTest.java b/services/tests/servicestests/src/com/android/server/audio/ServiceHolderTest.java
new file mode 100644
index 0000000..39f19ae
--- /dev/null
+++ b/services/tests/servicestests/src/com/android/server/audio/ServiceHolderTest.java
@@ -0,0 +1,284 @@
+/*
+ * 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.server.audio;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import android.media.IAudioPolicyService;
+import android.os.Binder;
+import android.os.IBinder;
+import android.os.IServiceCallback;
+import android.os.RemoteException;
+import android.platform.test.annotations.Presubmit;
+
+import androidx.test.ext.junit.runners.AndroidJUnit4;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.ArgumentCaptor;
+import org.mockito.ArgumentMatchers;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnit;
+import org.mockito.junit.MockitoRule;
+
+import java.util.concurrent.Executor;
+import java.util.function.Consumer;
+import java.util.function.Function;
+
+@RunWith(AndroidJUnit4.class)
+@Presubmit
+public class ServiceHolderTest {
+
+ private static final String AUDIO_POLICY_SERVICE_NAME = "media.audio_policy";
+
+ @Rule
+ public final MockitoRule mockito = MockitoJUnit.rule();
+
+ // the actual class under test
+ private ServiceHolder<IAudioPolicyService> mServiceHolder;
+
+ @Mock private ServiceHolder.ServiceProviderFacade mServiceProviderFacade;
+
+ @Mock private IAudioPolicyService mAudioPolicyService;
+
+ @Mock private IBinder mBinder;
+
+ @Mock private Consumer<IAudioPolicyService> mTaskOne;
+ @Mock private Consumer<IAudioPolicyService> mTaskTwo;
+
+ @Before
+ public void setUp() throws Exception {
+ mServiceHolder =
+ new ServiceHolder(
+ AUDIO_POLICY_SERVICE_NAME,
+ (Function<IBinder, IAudioPolicyService>)
+ binder -> {
+ if (binder == mBinder) {
+ return mAudioPolicyService;
+ } else {
+ return mock(IAudioPolicyService.class);
+ }
+ },
+ r -> r.run(),
+ mServiceProviderFacade);
+ when(mAudioPolicyService.asBinder()).thenReturn(mBinder);
+ }
+
+ @Test
+ public void testListenerRegistered_whenConstructed() {
+ verify(mServiceProviderFacade)
+ .registerForNotifications(eq(AUDIO_POLICY_SERVICE_NAME), ArgumentMatchers.any());
+ }
+
+ @Test
+ public void testServiceSuccessfullyPopulated_whenCallback() throws RemoteException {
+ initializeViaCallback();
+ verify(mBinder).linkToDeath(any(), anyInt());
+ assertThat(mServiceHolder.checkService()).isEqualTo(mAudioPolicyService);
+ }
+
+ @Test
+ public void testCheckServiceCalled_whenUncached() {
+ when(mServiceProviderFacade.checkService(eq(AUDIO_POLICY_SERVICE_NAME)))
+ .thenReturn(mBinder);
+ assertThat(mServiceHolder.checkService()).isEqualTo(mAudioPolicyService);
+ }
+
+ @Test
+ public void testCheckServiceTransmitsNull() {
+ assertThat(mServiceHolder.checkService()).isEqualTo(null);
+ }
+
+ @Test
+ public void testWaitForServiceCalled_whenUncached() {
+ when(mServiceProviderFacade.waitForService(eq(AUDIO_POLICY_SERVICE_NAME)))
+ .thenReturn(mBinder);
+ assertThat(mServiceHolder.waitForService()).isEqualTo(mAudioPolicyService);
+ }
+
+ @Test
+ public void testCheckServiceNotCalled_whenCached() {
+ initializeViaCallback();
+ mServiceHolder.checkService();
+ verify(mServiceProviderFacade, never()).checkService(any());
+ }
+
+ @Test
+ public void testWaitForServiceNotCalled_whenCached() {
+ initializeViaCallback();
+ mServiceHolder.waitForService();
+ verify(mServiceProviderFacade, never()).waitForService(any());
+ }
+
+ @Test
+ public void testStartTaskCalled_onStart() {
+ mServiceHolder.registerOnStartTask(mTaskOne);
+ mServiceHolder.registerOnStartTask(mTaskTwo);
+ mServiceHolder.unregisterOnStartTask(mTaskOne);
+ when(mServiceProviderFacade.checkService(eq(AUDIO_POLICY_SERVICE_NAME)))
+ .thenReturn(mBinder);
+
+ assertThat(mServiceHolder.checkService()).isEqualTo(mAudioPolicyService);
+
+ verify(mTaskTwo).accept(eq(mAudioPolicyService));
+ verify(mTaskOne, never()).accept(any());
+ }
+
+ @Test
+ public void testStartTaskCalled_onStartFromCallback() {
+ mServiceHolder.registerOnStartTask(mTaskOne);
+ mServiceHolder.registerOnStartTask(mTaskTwo);
+ mServiceHolder.unregisterOnStartTask(mTaskOne);
+
+ initializeViaCallback();
+
+ assertThat(mServiceHolder.checkService()).isEqualTo(mAudioPolicyService);
+ verify(mTaskTwo).accept(eq(mAudioPolicyService));
+ verify(mTaskOne, never()).accept(any());
+ }
+
+ @Test
+ public void testStartTaskCalled_onRegisterAfterStarted() {
+ initializeViaCallback();
+ mServiceHolder.registerOnStartTask(mTaskOne);
+ verify(mTaskOne).accept(eq(mAudioPolicyService));
+ }
+
+ @Test
+ public void testBinderDied_clearsServiceAndUnlinks() {
+ initializeViaCallback();
+ assertThat(mServiceHolder.checkService()).isEqualTo(mAudioPolicyService);
+
+ mServiceHolder.binderDied(mBinder);
+
+ verify(mBinder).unlinkToDeath(any(), anyInt());
+ assertThat(mServiceHolder.checkService()).isEqualTo(null);
+ verify(mServiceProviderFacade).checkService(eq(AUDIO_POLICY_SERVICE_NAME));
+ }
+
+ @Test
+ public void testBinderDied_callsDeathTasks() {
+ mServiceHolder.registerOnDeathTask(mTaskOne);
+ mServiceHolder.registerOnDeathTask(mTaskTwo);
+ initializeViaCallback();
+ assertThat(mServiceHolder.checkService()).isEqualTo(mAudioPolicyService);
+ mServiceHolder.unregisterOnDeathTask(mTaskOne);
+
+ mServiceHolder.binderDied(mBinder);
+
+ verify(mTaskTwo).accept(eq(mAudioPolicyService));
+ verify(mTaskOne, never()).accept(any());
+ }
+
+ @Test
+ public void testAttemptClear_clearsServiceAndUnlinks() {
+ initializeViaCallback();
+ assertThat(mServiceHolder.checkService()).isEqualTo(mAudioPolicyService);
+
+ mServiceHolder.attemptClear(mBinder);
+
+ verify(mBinder).unlinkToDeath(any(), anyInt());
+ assertThat(mServiceHolder.checkService()).isEqualTo(null);
+ verify(mServiceProviderFacade).checkService(eq(AUDIO_POLICY_SERVICE_NAME));
+ }
+
+ @Test
+ public void testAttemptClear_callsDeathTasks() {
+ mServiceHolder.registerOnDeathTask(mTaskOne);
+ mServiceHolder.registerOnDeathTask(mTaskTwo);
+ initializeViaCallback();
+ assertThat(mServiceHolder.checkService()).isEqualTo(mAudioPolicyService);
+ mServiceHolder.unregisterOnDeathTask(mTaskOne);
+
+ mServiceHolder.attemptClear(mBinder);
+
+ verify(mTaskTwo).accept(eq(mAudioPolicyService));
+ verify(mTaskOne, never()).accept(any());
+ }
+
+ @Test
+ public void testSet_whenServiceSet_isIgnored() {
+ mServiceHolder.registerOnStartTask(mTaskOne);
+ when(mServiceProviderFacade.checkService(eq(AUDIO_POLICY_SERVICE_NAME)))
+ .thenReturn(mBinder);
+ assertThat(mServiceHolder.checkService()).isEqualTo(mAudioPolicyService);
+
+ verify(mTaskOne).accept(eq(mAudioPolicyService));
+
+ // get the callback
+ ArgumentCaptor<IServiceCallback> cb = ArgumentCaptor.forClass(IServiceCallback.class);
+ verify(mServiceProviderFacade)
+ .registerForNotifications(eq(AUDIO_POLICY_SERVICE_NAME), cb.capture());
+
+ // Simulate a service callback with a different instance
+ try {
+ cb.getValue().onRegistration(AUDIO_POLICY_SERVICE_NAME, new Binder());
+ } catch (RemoteException e) {
+ throw new RuntimeException(e);
+ }
+
+ // No additional start task call (i.e. only the first verify)
+ verify(mTaskOne).accept(any());
+ // Same instance
+ assertThat(mServiceHolder.checkService()).isEqualTo(mAudioPolicyService);
+
+ }
+
+ @Test
+ public void testClear_whenServiceCleared_isIgnored() {
+ mServiceHolder.registerOnDeathTask(mTaskOne);
+ mServiceHolder.attemptClear(mBinder);
+ verify(mTaskOne, never()).accept(any());
+ }
+
+ @Test
+ public void testClear_withDifferentCookie_isIgnored() {
+ mServiceHolder.registerOnDeathTask(mTaskOne);
+ initializeViaCallback();
+ assertThat(mServiceHolder.checkService()).isEqualTo(mAudioPolicyService);
+
+ // Notif for stale cookie
+ mServiceHolder.attemptClear(new Binder());
+
+ // Service shouldn't be cleared
+ assertThat(mServiceHolder.checkService()).isEqualTo(mAudioPolicyService);
+ // No death tasks should fire
+ verify(mTaskOne, never()).accept(any());
+ }
+
+ private void initializeViaCallback() {
+ ArgumentCaptor<IServiceCallback> cb = ArgumentCaptor.forClass(IServiceCallback.class);
+ verify(mServiceProviderFacade)
+ .registerForNotifications(eq(AUDIO_POLICY_SERVICE_NAME), cb.capture());
+
+ try {
+ cb.getValue().onRegistration(AUDIO_POLICY_SERVICE_NAME, mBinder);
+ } catch (RemoteException e) {
+ throw new RuntimeException(e);
+ }
+ }
+}