diff --git a/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerHalConcurrentCaptureHandler.java b/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerHalConcurrentCaptureHandler.java index c0ab65a3215c8..05d92beed11f9 100644 --- a/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerHalConcurrentCaptureHandler.java +++ b/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerHalConcurrentCaptureHandler.java @@ -17,7 +17,6 @@ package com.android.server.soundtrigger_middleware; import android.annotation.NonNull; -import android.media.permission.SafeCloseable; import android.media.soundtrigger.ModelParameterRange; import android.media.soundtrigger.PhraseRecognitionEvent; import android.media.soundtrigger.PhraseSoundModel; @@ -30,6 +29,7 @@ import android.media.soundtrigger.Status; import android.os.IBinder; import java.util.HashSet; +import java.util.Iterator; import java.util.LinkedList; import java.util.Map; import java.util.Queue; @@ -63,18 +63,24 @@ import java.util.concurrent.ConcurrentHashMap; */ public class SoundTriggerHalConcurrentCaptureHandler implements ISoundTriggerHal, ICaptureStateNotifier.Listener { - private final @NonNull ISoundTriggerHal mDelegate; + @NonNull private final ISoundTriggerHal mDelegate; private GlobalCallback mGlobalCallback; + /** + * This lock must be held to synchronize forward calls (start/stop/onCaptureStateChange) that + * update the mActiveModels set and mCaptureState. + * It must not be locked in HAL callbacks to avoid deadlocks. + */ + @NonNull private final Object mStartStopLock = new Object(); /** * Information about a model that is currently loaded. This is needed in order to be able to * send abort events to its designated callback. */ private static class LoadedModel { - final int type; - final @NonNull ModelCallback callback; + public final int type; + @NonNull public final ModelCallback callback; - private LoadedModel(int type, @NonNull ModelCallback callback) { + LoadedModel(int type, @NonNull ModelCallback callback) { this.type = type; this.callback = callback; } @@ -83,19 +89,19 @@ public class SoundTriggerHalConcurrentCaptureHandler implements ISoundTriggerHal /** * This map holds the model type for every model that is loaded. */ - private final @NonNull Map mLoadedModels = new ConcurrentHashMap<>(); + @NonNull private final Map mLoadedModels = new ConcurrentHashMap<>(); /** * A set of all models that are currently active. * We use this in order to know which models to stop in case of external capture. * Used as a lock to synchronize operations that effect activity. */ - private final @NonNull Set mActiveModels = new HashSet<>(); + @NonNull private final Set mActiveModels = new HashSet<>(); /** * Notifier for changes in capture state. */ - private final @NonNull ICaptureStateNotifier mNotifier; + @NonNull private final ICaptureStateNotifier mNotifier; /** * Whether capture is active. @@ -106,10 +112,10 @@ public class SoundTriggerHalConcurrentCaptureHandler implements ISoundTriggerHal * Since we're wrapping the death recipient, we need to keep a translation map for unlinking. * Key is the client recipient, value is the wrapper. */ - private final @NonNull Map + @NonNull private final Map mDeathRecipientMap = new ConcurrentHashMap<>(); - private final @NonNull CallbackThread mCallbackThread = new CallbackThread(); + @NonNull private final CallbackThread mCallbackThread = new CallbackThread(); public SoundTriggerHalConcurrentCaptureHandler( @NonNull ISoundTriggerHal delegate, @@ -122,20 +128,28 @@ public class SoundTriggerHalConcurrentCaptureHandler implements ISoundTriggerHal @Override public void startRecognition(int modelHandle, int deviceHandle, int ioHandle, RecognitionConfig config) { - synchronized (mActiveModels) { - if (mCaptureState) { - throw new RecoverableException(Status.RESOURCE_CONTENTION); + synchronized (mStartStopLock) { + synchronized (mActiveModels) { + if (mCaptureState) { + throw new RecoverableException(Status.RESOURCE_CONTENTION); + } + mDelegate.startRecognition(modelHandle, deviceHandle, ioHandle, config); + mActiveModels.add(modelHandle); } - mDelegate.startRecognition(modelHandle, deviceHandle, ioHandle, config); - mActiveModels.add(modelHandle); } } @Override public void stopRecognition(int modelHandle) { - synchronized (mActiveModels) { - mDelegate.stopRecognition(modelHandle); - mActiveModels.remove(modelHandle); + synchronized (mStartStopLock) { + boolean wasActive; + synchronized (mActiveModels) { + wasActive = mActiveModels.remove(modelHandle); + } + if (wasActive) { + // Must be done outside of the lock, since it may trigger synchronous callbacks. + mDelegate.stopRecognition(modelHandle); + } } // Block until all previous events are delivered. Since this is potentially blocking on // upward calls, it must be done outside the lock. @@ -144,27 +158,38 @@ public class SoundTriggerHalConcurrentCaptureHandler implements ISoundTriggerHal @Override public void onCaptureStateChange(boolean active) { - synchronized (mActiveModels) { + synchronized (mStartStopLock) { if (active) { - // Abort all active models. This must be done as one transaction to the event - // thread, in order to be able to dedupe events before they are delivered. - try (SafeCloseable ignored = mCallbackThread.stallReader()) { - for (int modelHandle : mActiveModels) { - mDelegate.stopRecognition(modelHandle); - LoadedModel model = mLoadedModels.get(modelHandle); - // An abort event must be the last one for its model. - mCallbackThread.pushWithDedupe(modelHandle, true, - () -> notifyAbort(modelHandle, model)); - } - } + abortAllActiveModels(); } else { - mGlobalCallback.onResourcesAvailable(); + if (mGlobalCallback != null) { + mGlobalCallback.onResourcesAvailable(); + } } - mCaptureState = active; } } + private void abortAllActiveModels() { + while (true) { + int toStop; + synchronized (mActiveModels) { + Iterator iterator = mActiveModels.iterator(); + if (!iterator.hasNext()) { + return; + } + toStop = iterator.next(); + mActiveModels.remove(toStop); + } + // Invoke stop outside of the lock. + mDelegate.stopRecognition(toStop); + + LoadedModel model = mLoadedModels.get(toStop); + // Queue an abort event (no need to flush). + mCallbackThread.push(() -> notifyAbort(toStop, model)); + } + } + @Override public int loadSoundModel(SoundModel soundModel, ModelCallback callback) { int handle = mDelegate.loadSoundModel(soundModel, new CallbackWrapper(callback)); @@ -188,23 +213,13 @@ public class SoundTriggerHalConcurrentCaptureHandler implements ISoundTriggerHal @Override public void registerCallback(GlobalCallback callback) { - mGlobalCallback = new GlobalCallback() { - @Override - public void onResourcesAvailable() { - mCallbackThread.push(callback::onResourcesAvailable); - } - }; + mGlobalCallback = () -> mCallbackThread.push(callback::onResourcesAvailable); mDelegate.registerCallback(mGlobalCallback); } @Override public void linkToDeath(IBinder.DeathRecipient recipient) { - IBinder.DeathRecipient wrapper = new IBinder.DeathRecipient() { - @Override - public void binderDied() { - mCallbackThread.push(() -> recipient.binderDied()); - } - }; + IBinder.DeathRecipient wrapper = () -> mCallbackThread.push(recipient::binderDied); mDelegate.linkToDeath(wrapper); mDeathRecipientMap.put(recipient, wrapper); } @@ -215,7 +230,7 @@ public class SoundTriggerHalConcurrentCaptureHandler implements ISoundTriggerHal } private class CallbackWrapper implements ISoundTriggerHal.ModelCallback { - private final @NonNull ISoundTriggerHal.ModelCallback mDelegateCallback; + @NonNull private final ISoundTriggerHal.ModelCallback mDelegateCallback; private CallbackWrapper(@NonNull ModelCallback delegateCallback) { mDelegateCallback = delegateCallback; @@ -223,18 +238,36 @@ public class SoundTriggerHalConcurrentCaptureHandler implements ISoundTriggerHal @Override public void recognitionCallback(int modelHandle, RecognitionEvent event) { - // A recognition event must be the last one for its model, unless it is a forced one - // (those leave the model active). - mCallbackThread.pushWithDedupe(modelHandle, !event.recognitionStillActive, - () -> mDelegateCallback.recognitionCallback(modelHandle, event)); + synchronized (mActiveModels) { + if (!mActiveModels.contains(modelHandle)) { + // Discard the event. + return; + } + if (!event.recognitionStillActive) { + mActiveModels.remove(modelHandle); + } + // A recognition event must be the last one for its model, unless it indicates that + // recognition is still active. + mCallbackThread.push( + () -> mDelegateCallback.recognitionCallback(modelHandle, event)); + } } @Override public void phraseRecognitionCallback(int modelHandle, PhraseRecognitionEvent event) { - // A recognition event must be the last one for its model, unless it is a forced one - // (those leave the model active). - mCallbackThread.pushWithDedupe(modelHandle, !event.common.recognitionStillActive, - () -> mDelegateCallback.phraseRecognitionCallback(modelHandle, event)); + synchronized (mActiveModels) { + if (!mActiveModels.contains(modelHandle)) { + // Discard the event. + return; + } + if (!event.common.recognitionStillActive) { + mActiveModels.remove(modelHandle); + } + // A recognition event must be the last one for its model, unless it indicates that + // recognition is still active. + mCallbackThread.push( + () -> mDelegateCallback.phraseRecognitionCallback(modelHandle, event)); + } } @Override @@ -254,36 +287,12 @@ public class SoundTriggerHalConcurrentCaptureHandler implements ISoundTriggerHal *
    *
  • Events are processed on a separate thread than the thread that pushed them, in the order * they were pushed. - *
  • Events can be deduped upon entry to the queue. This is achieved as follows: - *
      - *
    • Temporarily stall the reader via {@link #stallReader()}. - *
    • Within this scope, push as many events as needed via - * {@link #pushWithDedupe(int, boolean, Runnable)}. - * If an event with the same model handle as the one being pushed is already in the queue - * and has been marked as "lastForModel", the new event will be discarded before entering - * the queue. - *
    • Finally, un-stall the reader by existing the scope. - *
    • Events that do not require deduping can be pushed via {@link #push(Runnable)}. - *
    *
  • Events can be flushed via {@link #flush()}. This will block until all events pushed prior * to this call have been fully processed. *
*/ private static class CallbackThread { - private static class Entry { - final boolean lastForModel; - final int modelHandle; - final Runnable runnable; - - private Entry(boolean lastForModel, int modelHandle, Runnable runnable) { - this.lastForModel = lastForModel; - this.modelHandle = modelHandle; - this.runnable = runnable; - } - } - - private boolean mStallReader = false; - private final Queue mList = new LinkedList<>(); + private final Queue mList = new LinkedList<>(); private int mPushCount = 0; private int mProcessedCount = 0; @@ -312,23 +321,11 @@ public class SoundTriggerHalConcurrentCaptureHandler implements ISoundTriggerHal * @param runnable The runnable to push. */ void push(Runnable runnable) { - pushEntry(new Entry(false, 0, runnable), false); - } - - - /** - * Push a new runnable to the queue, with deduping. - * If an entry with the same model handle is already in the queue and was designated as - * last for model, this one will be discarded. - * - * @param modelHandle The model handle, used for deduping purposes. - * @param lastForModel If true, this entry will be considered the last one for this model - * and any subsequence calls for this handle (whether lastForModel or - * not) will be discarded while this entry is in the queue. - * @param runnable The runnable to push. - */ - void pushWithDedupe(int modelHandle, boolean lastForModel, Runnable runnable) { - pushEntry(new Entry(lastForModel, modelHandle, runnable), true); + synchronized (mList) { + mList.add(runnable); + mPushCount++; + mList.notifyAll(); + } } /** @@ -346,45 +343,15 @@ public class SoundTriggerHalConcurrentCaptureHandler implements ISoundTriggerHal } } - /** - * Creates a scope (using a try-with-resources block), within which events that are pushed - * remain queued and processed. This is useful in order to utilize deduping. - */ - SafeCloseable stallReader() { - synchronized (mList) { - mStallReader = true; - return () -> { - synchronized (mList) { - mStallReader = false; - mList.notifyAll(); - } - }; - } - } - - private void pushEntry(Entry entry, boolean dedupe) { - synchronized (mList) { - if (dedupe) { - for (Entry existing : mList) { - if (existing.lastForModel && existing.modelHandle == entry.modelHandle) { - return; - } - } - } - mList.add(entry); - mPushCount++; - mList.notifyAll(); - } - } - private Runnable pop() throws InterruptedException { synchronized (mList) { - while (mStallReader || mList.isEmpty()) { + while (mList.isEmpty()) { mList.wait(); } - return mList.remove().runnable; + return mList.remove(); } } + } /** Notify the client that recognition has been aborted. */ diff --git a/services/tests/servicestests/src/com/android/server/soundtrigger_middleware/SoundHw2CompatTest.java b/services/tests/servicestests/src/com/android/server/soundtrigger_middleware/SoundHw2CompatTest.java index 6bdd88c6a7126..2d0755d00ba8e 100644 --- a/services/tests/servicestests/src/com/android/server/soundtrigger_middleware/SoundHw2CompatTest.java +++ b/services/tests/servicestests/src/com/android/server/soundtrigger_middleware/SoundHw2CompatTest.java @@ -20,18 +20,15 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.fail; -import static org.junit.Assume.assumeFalse; import static org.junit.Assume.assumeTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.atLeast; import static org.mockito.Mockito.atMost; import static org.mockito.Mockito.clearInvocations; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; @@ -56,32 +53,20 @@ import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import org.mockito.ArgumentCaptor; -import java.util.LinkedList; -import java.util.List; import java.util.concurrent.atomic.AtomicReference; @RunWith(Parameterized.class) public class SoundHw2CompatTest { - @Parameterized.Parameter(0) public String mVersion; - @Parameterized.Parameter(1) public boolean mSupportConcurrentCapture; + @Parameterized.Parameter public String mVersion; private final Runnable mRebootRunnable = mock(Runnable.class); private ISoundTriggerHal mCanonical; - private CaptureStateNotifier mCaptureStateNotifier; private android.hardware.soundtrigger.V2_0.ISoundTriggerHw mHalDriver; // We run the test once for every version of the underlying driver. - @Parameterized.Parameters(name = "{0}, concurrent={1}") - public static Iterable data() { - List result = new LinkedList<>(); - - for (String version : new String[]{"V2_0", "V2_1", "V2_2", "V2_3",}) { - for (boolean concurrentCapture : new boolean[]{false, true}) { - result.add(new Object[]{version, concurrentCapture}); - } - } - - return result; + @Parameterized.Parameters + public static Object[] data() { + return new String[]{"V2_0", "V2_1", "V2_2", "V2_3"}; } @Before @@ -139,7 +124,7 @@ public class SoundHw2CompatTest { when(mHalDriver.asBinder()).thenReturn(binder); android.hardware.soundtrigger.V2_3.Properties halProperties = - TestUtil.createDefaultProperties_2_3(mSupportConcurrentCapture); + TestUtil.createDefaultProperties_2_3(); doAnswer(invocation -> { ((android.hardware.soundtrigger.V2_0.ISoundTriggerHw.getPropertiesCallback) invocation.getArgument( 0)).onValues(0, halProperties.base); @@ -156,10 +141,7 @@ public class SoundHw2CompatTest { }).when(driver).getProperties_2_3(any()); } - mCaptureStateNotifier = spy(new CaptureStateNotifier()); - - mCanonical = SoundTriggerHw2Compat.create(mHalDriver, mRebootRunnable, - mCaptureStateNotifier); + mCanonical = SoundTriggerHw2Compat.create(mHalDriver, mRebootRunnable, null); // During initialization any method can be called, but after we're starting to enforce that // no additional methods are called. @@ -171,7 +153,6 @@ public class SoundHw2CompatTest { mCanonical.detach(); verifyNoMoreInteractions(mHalDriver); verifyNoMoreInteractions(mRebootRunnable); - mCaptureStateNotifier.verifyNoMoreListeners(); } @Test @@ -194,12 +175,12 @@ public class SoundHw2CompatTest { // It is OK for the SUT to cache the properties, so the underlying method doesn't // need to be called every single time. verify(driver, atMost(1)).getProperties_2_3(any()); - TestUtil.validateDefaultProperties(properties, mSupportConcurrentCapture); + TestUtil.validateDefaultProperties(properties); } else { // It is OK for the SUT to cache the properties, so the underlying method doesn't // need to be called every single time. verify(mHalDriver, atMost(1)).getProperties(any()); - TestUtil.validateDefaultProperties(properties, mSupportConcurrentCapture, 0, ""); + TestUtil.validateDefaultProperties(properties, 0, ""); } } @@ -291,7 +272,7 @@ public class SoundHw2CompatTest { ISoundTriggerHal.ModelCallback canonicalCallback = mock( ISoundTriggerHal.ModelCallback.class); - final int maxModels = TestUtil.createDefaultProperties_2_0(false).maxSoundModels; + final int maxModels = TestUtil.createDefaultProperties_2_0().maxSoundModels; int[] modelHandles = new int[maxModels]; // Load as many models as we're allowed. @@ -318,7 +299,7 @@ public class SoundHw2CompatTest { verify(globalCallback).onResourcesAvailable(); } - private int loadPhraseModel_2_0(ISoundTriggerHal.ModelCallback canonicalCallback) + private void loadPhraseModel_2_0(ISoundTriggerHal.ModelCallback canonicalCallback) throws Exception { final int handle = 29; ArgumentCaptor @@ -345,10 +326,9 @@ public class SoundHw2CompatTest { TestUtil.validatePhraseSoundModel_2_0(modelCaptor.getValue()); validateCallback_2_0(callbackCaptor.getValue(), canonicalCallback); - return handle; } - private int loadPhraseModel_2_1(ISoundTriggerHal.ModelCallback canonicalCallback) + private void loadPhraseModel_2_1(ISoundTriggerHal.ModelCallback canonicalCallback) throws Exception { final android.hardware.soundtrigger.V2_1.ISoundTriggerHw driver_2_1 = (android.hardware.soundtrigger.V2_1.ISoundTriggerHw) mHalDriver; @@ -380,14 +360,13 @@ public class SoundHw2CompatTest { TestUtil.validatePhraseSoundModel_2_1(model.get()); validateCallback_2_1(callbackCaptor.getValue(), canonicalCallback); - return handle; } - public int loadPhraseModel(ISoundTriggerHal.ModelCallback canonicalCallback) throws Exception { + public void loadPhraseModel(ISoundTriggerHal.ModelCallback canonicalCallback) throws Exception { if (mHalDriver instanceof android.hardware.soundtrigger.V2_1.ISoundTriggerHw) { - return loadPhraseModel_2_1(canonicalCallback); + loadPhraseModel_2_1(canonicalCallback); } else { - return loadPhraseModel_2_0(canonicalCallback); + loadPhraseModel_2_0(canonicalCallback); } } @@ -483,80 +462,6 @@ public class SoundHw2CompatTest { startRecognition(handle, canonicalCallback); } - @Test - public void testConcurrentCaptureAbort() throws Exception { - assumeFalse(mSupportConcurrentCapture); - verify(mCaptureStateNotifier, atLeast(1)).registerListener(any()); - - // Register global callback. - ISoundTriggerHal.GlobalCallback globalCallback = mock( - ISoundTriggerHal.GlobalCallback.class); - mCanonical.registerCallback(globalCallback); - - // Load. - ISoundTriggerHal.ModelCallback canonicalCallback = mock( - ISoundTriggerHal.ModelCallback.class); - final int handle = loadGenericModel(canonicalCallback); - - // Then start. - startRecognition(handle, canonicalCallback); - - // Now activate external capture. - mCaptureStateNotifier.setState(true); - - // Expect hardware to have been stopped. - verify(mHalDriver).stopRecognition(handle); - - // Expect an abort event (async). - ArgumentCaptor eventCaptor = ArgumentCaptor.forClass( - RecognitionEvent.class); - mCanonical.flushCallbacks(); - verify(canonicalCallback).recognitionCallback(eq(handle), eventCaptor.capture()); - assertEquals(RecognitionStatus.ABORTED, eventCaptor.getValue().status); - - // Deactivate external capture. - mCaptureStateNotifier.setState(false); - - // Expect a onResourcesAvailable(). - mCanonical.flushCallbacks(); - verify(globalCallback).onResourcesAvailable(); - } - - @Test - public void testConcurrentCaptureReject() throws Exception { - assumeFalse(mSupportConcurrentCapture); - verify(mCaptureStateNotifier, atLeast(1)).registerListener(any()); - - // Register global callback. - ISoundTriggerHal.GlobalCallback globalCallback = mock( - ISoundTriggerHal.GlobalCallback.class); - mCanonical.registerCallback(globalCallback); - - // Load (this registers the callback). - ISoundTriggerHal.ModelCallback canonicalCallback = mock( - ISoundTriggerHal.ModelCallback.class); - final int handle = loadGenericModel(canonicalCallback); - - // Report external capture active. - mCaptureStateNotifier.setState(true); - - // Then start. - RecognitionConfig config = TestUtil.createRecognitionConfig(); - try { - mCanonical.startRecognition(handle, 203, 204, config); - fail("Expected an exception"); - } catch (RecoverableException e) { - assertEquals(Status.RESOURCE_CONTENTION, e.errorCode); - } - - // Deactivate external capture. - mCaptureStateNotifier.setState(false); - - // Expect a onResourcesAvailable(). - mCanonical.flushCallbacks(); - verify(globalCallback).onResourcesAvailable(); - } - @Test public void testStopRecognition() throws Exception { mCanonical.stopRecognition(17); @@ -675,7 +580,7 @@ public class SoundHw2CompatTest { } @Test - public void testGlobalCallback() throws Exception { + public void testGlobalCallback() { testGlobalCallback_2_0(); } @@ -803,29 +708,4 @@ public class SoundHw2CompatTest { verifyNoMoreInteractions(canonicalCallback); clearInvocations(canonicalCallback); } - - public static class CaptureStateNotifier implements ICaptureStateNotifier { - private final List mListeners = new LinkedList<>(); - - @Override - public boolean registerListener(Listener listener) { - mListeners.add(listener); - return false; - } - - @Override - public void unregisterListener(Listener listener) { - mListeners.remove(listener); - } - - public void setState(boolean state) { - for (Listener listener : mListeners) { - listener.onCaptureStateChange(state); - } - } - - public void verifyNoMoreListeners() { - assertEquals(0, mListeners.size()); - } - } } diff --git a/services/tests/servicestests/src/com/android/server/soundtrigger_middleware/SoundTriggerHalConcurrentCaptureHandlerTest.java b/services/tests/servicestests/src/com/android/server/soundtrigger_middleware/SoundTriggerHalConcurrentCaptureHandlerTest.java new file mode 100644 index 0000000000000..61989252d04dc --- /dev/null +++ b/services/tests/servicestests/src/com/android/server/soundtrigger_middleware/SoundTriggerHalConcurrentCaptureHandlerTest.java @@ -0,0 +1,313 @@ +/* + * Copyright (C) 2022 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.soundtrigger_middleware; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.atMost; +import static org.mockito.Mockito.clearInvocations; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.verifyZeroInteractions; + +import android.media.soundtrigger.RecognitionEvent; +import android.media.soundtrigger.RecognitionStatus; + +import androidx.annotation.NonNull; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; +import org.mockito.ArgumentCaptor; + +@RunWith(JUnit4.class) +public class SoundTriggerHalConcurrentCaptureHandlerTest { + private ISoundTriggerHal mUnderlying; + private CaptureStateNotifier mNotifier; + private ISoundTriggerHal.GlobalCallback mGlobalCallback; + private SoundTriggerHalConcurrentCaptureHandler mHandler; + + @Before + public void setUp() { + mNotifier = new CaptureStateNotifier(); + mUnderlying = mock(ISoundTriggerHal.class); + mGlobalCallback = mock(ISoundTriggerHal.GlobalCallback.class); + mHandler = new SoundTriggerHalConcurrentCaptureHandler(mUnderlying, mNotifier); + mHandler.registerCallback(mGlobalCallback); + } + + @Test + public void testBasic() throws Exception { + ISoundTriggerHal.ModelCallback callback = mock(ISoundTriggerHal.ModelCallback.class); + int handle = mHandler.loadSoundModel(TestUtil.createGenericSoundModel(), callback); + verify(mUnderlying).loadSoundModel(any(), any()); + + mHandler.startRecognition(handle, 101, 102, TestUtil.createRecognitionConfig()); + verify(mUnderlying).startRecognition(eq(handle), eq(101), eq(102), any()); + + mNotifier.setActive(true); + verify(mUnderlying).stopRecognition(handle); + ArgumentCaptor eventCaptor = ArgumentCaptor.forClass( + RecognitionEvent.class); + Thread.sleep(50); + verify(callback).recognitionCallback(eq(handle), eventCaptor.capture()); + RecognitionEvent event = eventCaptor.getValue(); + assertEquals(event.status, RecognitionStatus.ABORTED); + assertFalse(event.recognitionStillActive); + verifyZeroInteractions(mGlobalCallback); + clearInvocations(callback, mUnderlying); + + mNotifier.setActive(false); + Thread.sleep(50); + verify(mGlobalCallback).onResourcesAvailable(); + verifyNoMoreInteractions(callback, mUnderlying); + + mNotifier.setActive(true); + verifyNoMoreInteractions(callback, mUnderlying); + } + + @Test + public void testStopBeforeActive() throws Exception { + ISoundTriggerHal.ModelCallback callback = mock(ISoundTriggerHal.ModelCallback.class); + int handle = mHandler.loadSoundModel(TestUtil.createGenericSoundModel(), callback); + verify(mUnderlying).loadSoundModel(any(), any()); + + mHandler.startRecognition(handle, 101, 102, TestUtil.createRecognitionConfig()); + verify(mUnderlying).startRecognition(eq(handle), eq(101), eq(102), any()); + mHandler.stopRecognition(handle); + verify(mUnderlying).stopRecognition(handle); + clearInvocations(mUnderlying); + + mNotifier.setActive(true); + Thread.sleep(50); + verifyNoMoreInteractions(mUnderlying); + verifyNoMoreInteractions(callback); + } + + @Test + public void testStopAfterActive() { + ISoundTriggerHal.ModelCallback callback = mock(ISoundTriggerHal.ModelCallback.class); + int handle = mHandler.loadSoundModel(TestUtil.createGenericSoundModel(), callback); + verify(mUnderlying).loadSoundModel(any(), any()); + + mHandler.startRecognition(handle, 101, 102, TestUtil.createRecognitionConfig()); + verify(mUnderlying).startRecognition(eq(handle), eq(101), eq(102), any()); + + mNotifier.setActive(true); + verify(mUnderlying, times(1)).stopRecognition(handle); + mHandler.stopRecognition(handle); + verify(callback, times(1)).recognitionCallback(eq(handle), any()); + } + + @Test(timeout = 200) + public void testAbortWhileStop() { + ISoundTriggerHal.ModelCallback callback = mock(ISoundTriggerHal.ModelCallback.class); + int handle = mHandler.loadSoundModel(TestUtil.createGenericSoundModel(), callback); + ArgumentCaptor modelCallbackCaptor = + ArgumentCaptor.forClass(ISoundTriggerHal.ModelCallback.class); + verify(mUnderlying).loadSoundModel(any(), modelCallbackCaptor.capture()); + ISoundTriggerHal.ModelCallback modelCallback = modelCallbackCaptor.getValue(); + + mHandler.startRecognition(handle, 101, 102, TestUtil.createRecognitionConfig()); + verify(mUnderlying).startRecognition(eq(handle), eq(101), eq(102), any()); + + doAnswer(invocation -> { + RecognitionEvent event = TestUtil.createRecognitionEvent(RecognitionStatus.ABORTED, + false); + // Call the callback from a different thread to detect deadlocks by preventing recursive + // locking from working. + runOnSeparateThread(() -> modelCallback.recognitionCallback(handle, event)); + return null; + }).when(mUnderlying).stopRecognition(handle); + mHandler.stopRecognition(handle); + verify(mUnderlying, times(1)).stopRecognition(handle); + + ArgumentCaptor eventCaptor = ArgumentCaptor.forClass( + RecognitionEvent.class); + verify(callback, atMost(1)).recognitionCallback(eq(handle), eventCaptor.capture()); + } + + @Test(timeout = 200) + public void testActiveWhileStop() { + ISoundTriggerHal.ModelCallback callback = mock(ISoundTriggerHal.ModelCallback.class); + int handle = mHandler.loadSoundModel(TestUtil.createGenericSoundModel(), callback); + ArgumentCaptor modelCallbackCaptor = + ArgumentCaptor.forClass(ISoundTriggerHal.ModelCallback.class); + verify(mUnderlying).loadSoundModel(any(), modelCallbackCaptor.capture()); + ISoundTriggerHal.ModelCallback modelCallback = modelCallbackCaptor.getValue(); + + mHandler.startRecognition(handle, 101, 102, TestUtil.createRecognitionConfig()); + verify(mUnderlying).startRecognition(eq(handle), eq(101), eq(102), any()); + + doAnswer(invocation -> { + // The stop request causes a callback to be flushed. + RecognitionEvent event = TestUtil.createRecognitionEvent(RecognitionStatus.FORCED, + true); + // Call the callback from a different thread to detect deadlocks by preventing recursive + // locking from working. + runOnSeparateThread(() -> modelCallback.recognitionCallback(handle, event)); + // While the HAL is processing the stop request, capture state becomes active. + new Thread(() -> mNotifier.setActive(true)).start(); + Thread.sleep(50); + return null; + }).when(mUnderlying).stopRecognition(handle); + mHandler.stopRecognition(handle); + // We only expect one underlying invocation of stop(). + verify(mUnderlying, times(1)).stopRecognition(handle); + + // The callback shouldn't be invoked in this case. + verify(callback, never()).recognitionCallback(eq(handle), any()); + } + + @Test(timeout = 200) + public void testStopWhileActive() { + ISoundTriggerHal.ModelCallback callback = mock(ISoundTriggerHal.ModelCallback.class); + int handle = mHandler.loadSoundModel(TestUtil.createGenericSoundModel(), callback); + ArgumentCaptor modelCallbackCaptor = + ArgumentCaptor.forClass(ISoundTriggerHal.ModelCallback.class); + verify(mUnderlying).loadSoundModel(any(), modelCallbackCaptor.capture()); + ISoundTriggerHal.ModelCallback modelCallback = modelCallbackCaptor.getValue(); + + mHandler.startRecognition(handle, 101, 102, TestUtil.createRecognitionConfig()); + verify(mUnderlying).startRecognition(eq(handle), eq(101), eq(102), any()); + + doAnswer(invocation -> { + // The stop request causes a callback to be flushed. + RecognitionEvent event = TestUtil.createRecognitionEvent(RecognitionStatus.FORCED, + true); + // Call the callback from a different thread to detect deadlocks by preventing recursive + // locking from working. + runOnSeparateThread(() -> modelCallback.recognitionCallback(handle, event)); + // While the HAL is processing the stop request, client requests stop. + new Thread(() -> mHandler.stopRecognition(handle)).start(); + Thread.sleep(50); + return null; + }).when(mUnderlying).stopRecognition(handle); + mNotifier.setActive(true); + // We only expect one underlying invocation of stop(). + verify(mUnderlying, times(1)).stopRecognition(handle); + verify(callback, atMost(1)).recognitionCallback(eq(handle), any()); + } + + @Test(timeout = 200) + public void testEventWhileActive() throws Exception { + ISoundTriggerHal.ModelCallback callback = mock(ISoundTriggerHal.ModelCallback.class); + int handle = mHandler.loadSoundModel(TestUtil.createGenericSoundModel(), callback); + ArgumentCaptor modelCallbackCaptor = + ArgumentCaptor.forClass(ISoundTriggerHal.ModelCallback.class); + verify(mUnderlying).loadSoundModel(any(), modelCallbackCaptor.capture()); + ISoundTriggerHal.ModelCallback modelCallback = modelCallbackCaptor.getValue(); + + mHandler.startRecognition(handle, 101, 102, TestUtil.createRecognitionConfig()); + verify(mUnderlying).startRecognition(eq(handle), eq(101), eq(102), any()); + + doAnswer(invocation -> { + RecognitionEvent event = TestUtil.createRecognitionEvent(RecognitionStatus.SUCCESS, + false); + // Call the callback from a different thread to detect deadlocks by preventing recursive + // locking from working. + runOnSeparateThread(() -> modelCallback.recognitionCallback(handle, event)); + return null; + }).when(mUnderlying).stopRecognition(handle); + mNotifier.setActive(true); + verify(mUnderlying, times(1)).stopRecognition(handle); + Thread.sleep(50); + + ArgumentCaptor eventCaptor = ArgumentCaptor.forClass( + RecognitionEvent.class); + verify(callback, atMost(2)).recognitionCallback(eq(handle), eventCaptor.capture()); + RecognitionEvent lastEvent = eventCaptor.getValue(); + assertEquals(lastEvent.status, RecognitionStatus.ABORTED); + assertFalse(lastEvent.recognitionStillActive); + } + + + @Test(timeout = 200) + public void testNonFinalEventWhileActive() throws Exception { + ISoundTriggerHal.ModelCallback callback = mock(ISoundTriggerHal.ModelCallback.class); + int handle = mHandler.loadSoundModel(TestUtil.createGenericSoundModel(), callback); + ArgumentCaptor modelCallbackCaptor = + ArgumentCaptor.forClass(ISoundTriggerHal.ModelCallback.class); + verify(mUnderlying).loadSoundModel(any(), modelCallbackCaptor.capture()); + ISoundTriggerHal.ModelCallback modelCallback = modelCallbackCaptor.getValue(); + + mHandler.startRecognition(handle, 101, 102, TestUtil.createRecognitionConfig()); + verify(mUnderlying).startRecognition(eq(handle), eq(101), eq(102), any()); + + doAnswer(invocation -> { + RecognitionEvent event = TestUtil.createRecognitionEvent(RecognitionStatus.FORCED, + true); + // Call the callback from a different thread to detect deadlocks by preventing recursive + // locking from working. + runOnSeparateThread(() -> modelCallback.recognitionCallback(handle, event)); + + return null; + }).when(mUnderlying).stopRecognition(handle); + mNotifier.setActive(true); + verify(mUnderlying, times(1)).stopRecognition(handle); + + Thread.sleep(50); + ArgumentCaptor eventCaptor = ArgumentCaptor.forClass( + RecognitionEvent.class); + verify(callback, atMost(2)).recognitionCallback(eq(handle), eventCaptor.capture()); + RecognitionEvent lastEvent = eventCaptor.getValue(); + assertEquals(lastEvent.status, RecognitionStatus.ABORTED); + assertFalse(lastEvent.recognitionStillActive); + } + + private static void runOnSeparateThread(Runnable runnable) { + Thread thread = new Thread(runnable); + thread.start(); + try { + thread.join(); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + } + + private static class CaptureStateNotifier implements ICaptureStateNotifier { + boolean mActive = false; + Listener mListener; + + @Override + public boolean registerListener(@NonNull Listener listener) { + mListener = listener; + return mActive; + } + + @Override + public void unregisterListener(@NonNull Listener listener) { + mListener = null; + } + + public void setActive(boolean active) { + mActive = active; + if (mListener != null) { + // Call the callback from a different thread to detect deadlocks by preventing + // recursive locking from working. + runOnSeparateThread(() -> mListener.onCaptureStateChange(mActive)); + } + } + } +} diff --git a/services/tests/servicestests/src/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareImplTest.java b/services/tests/servicestests/src/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareImplTest.java index 0187e34cbc5f9..3bebc94fe0edb 100644 --- a/services/tests/servicestests/src/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareImplTest.java +++ b/services/tests/servicestests/src/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareImplTest.java @@ -134,7 +134,7 @@ public class SoundTriggerMiddlewareImplTest { public void setUp() throws Exception { clearInvocations(mHalDriver); clearInvocations(mAudioSessionProvider); - when(mHalDriver.getProperties()).thenReturn(TestUtil.createDefaultProperties(false)); + when(mHalDriver.getProperties()).thenReturn(TestUtil.createDefaultProperties()); mService = new SoundTriggerMiddlewareImpl(() -> mHalDriver, mAudioSessionProvider); } @@ -156,7 +156,7 @@ public class SoundTriggerMiddlewareImplTest { assertEquals(1, allDescriptors.length); Properties properties = allDescriptors[0].properties; - assertEquals(TestUtil.createDefaultProperties(false), properties); + assertEquals(TestUtil.createDefaultProperties(), properties); } @Test diff --git a/services/tests/servicestests/src/com/android/server/soundtrigger_middleware/TestUtil.java b/services/tests/servicestests/src/com/android/server/soundtrigger_middleware/TestUtil.java index 30b4a59b32b1a..39561f74d7edc 100644 --- a/services/tests/servicestests/src/com/android/server/soundtrigger_middleware/TestUtil.java +++ b/services/tests/servicestests/src/com/android/server/soundtrigger_middleware/TestUtil.java @@ -162,8 +162,8 @@ class TestUtil { phrases.get(0).recognitionModes); } - static android.hardware.soundtrigger.V2_0.ISoundTriggerHw.Properties createDefaultProperties_2_0( - boolean supportConcurrentCapture) { + static android.hardware.soundtrigger.V2_0.ISoundTriggerHw.Properties + createDefaultProperties_2_0() { android.hardware.soundtrigger.V2_0.ISoundTriggerHw.Properties properties = new android.hardware.soundtrigger.V2_0.ISoundTriggerHw.Properties(); properties.implementor = "implementor"; @@ -185,17 +185,16 @@ class TestUtil { | android.hardware.soundtrigger.V2_0.RecognitionMode.GENERIC_TRIGGER; properties.captureTransition = true; properties.maxBufferMs = 321; - properties.concurrentCapture = supportConcurrentCapture; + properties.concurrentCapture = true; properties.triggerInEvent = true; properties.powerConsumptionMw = 432; return properties; } - static android.hardware.soundtrigger.V2_3.Properties createDefaultProperties_2_3( - boolean supportConcurrentCapture) { + static android.hardware.soundtrigger.V2_3.Properties createDefaultProperties_2_3() { android.hardware.soundtrigger.V2_3.Properties properties = new android.hardware.soundtrigger.V2_3.Properties(); - properties.base = createDefaultProperties_2_0(supportConcurrentCapture); + properties.base = createDefaultProperties_2_0(); properties.supportedModelArch = "supportedModelArch"; properties.audioCapabilities = android.hardware.soundtrigger.V2_3.AudioCapabilities.ECHO_CANCELLATION @@ -203,7 +202,7 @@ class TestUtil { return properties; } - static Properties createDefaultProperties(boolean supportConcurrentCapture) { + static Properties createDefaultProperties() { Properties properties = new Properties(); properties.implementor = "implementor"; properties.description = "description"; @@ -217,7 +216,7 @@ class TestUtil { | RecognitionMode.USER_AUTHENTICATION | RecognitionMode.GENERIC_TRIGGER; properties.captureTransition = true; properties.maxBufferMs = 321; - properties.concurrentCapture = supportConcurrentCapture; + properties.concurrentCapture = true; properties.triggerInEvent = true; properties.powerConsumptionMw = 432; properties.supportedModelArch = "supportedModelArch"; @@ -226,13 +225,13 @@ class TestUtil { return properties; } - static void validateDefaultProperties(Properties properties, boolean supportConcurrentCapture) { - validateDefaultProperties(properties, supportConcurrentCapture, + static void validateDefaultProperties(Properties properties) { + validateDefaultProperties(properties, AudioCapabilities.ECHO_CANCELLATION | AudioCapabilities.NOISE_SUPPRESSION, "supportedModelArch"); } - static void validateDefaultProperties(Properties properties, boolean supportConcurrentCapture, + static void validateDefaultProperties(Properties properties, @AudioCapabilities int audioCapabilities, @NonNull String supportedModelArch) { assertEquals("implementor", properties.implementor); assertEquals("description", properties.description); @@ -246,7 +245,7 @@ class TestUtil { properties.recognitionModes); assertTrue(properties.captureTransition); assertEquals(321, properties.maxBufferMs); - assertEquals(supportConcurrentCapture, properties.concurrentCapture); + assertEquals(true, properties.concurrentCapture); assertTrue(properties.triggerInEvent); assertEquals(432, properties.powerConsumptionMw); assertEquals(supportedModelArch, properties.supportedModelArch);