From 6e2eb81ac4687ac5340673ff996c0591154def7f Mon Sep 17 00:00:00 2001 From: Ytai Ben-Tsvi Date: Tue, 15 Mar 2022 11:34:18 -0700 Subject: [PATCH] Wait for an abort event when stopping a model Because the client callbacks from SoundTriggerMiddleware service are async, if a client quickly stops and starts a model, then receives a detection event for that model, it is impossible for it to tell whether the event corresponding to the previous or current session, and thus is unable to reason about the resulting state of the model. To mitigate that, we will always wait for an abort event when a model is stopped before any other operation is attempted. This way the model state stays synchronized. Fixes: 191935600 Test: Manual verification of soundtrigger use-cases. Specifically, quick toggling of Assitant and Now Playing via Settings. Change-Id: I010558e8fa1891922f20dcdb7b0ca04c9500c2e1 --- .../soundtrigger/SoundTriggerHelper.java | 82 ++++++++++++------- 1 file changed, 53 insertions(+), 29 deletions(-) diff --git a/services/voiceinteraction/java/com/android/server/soundtrigger/SoundTriggerHelper.java b/services/voiceinteraction/java/com/android/server/soundtrigger/SoundTriggerHelper.java index 24ce7e76a49c6..c8bcb83cae74e 100644 --- a/services/voiceinteraction/java/com/android/server/soundtrigger/SoundTriggerHelper.java +++ b/services/voiceinteraction/java/com/android/server/soundtrigger/SoundTriggerHelper.java @@ -84,6 +84,9 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { private static final int INVALID_VALUE = Integer.MIN_VALUE; + /** Maximum time to wait for a model stop confirmation before giving up. */ + private static final long STOP_TIMEOUT_MS = 5000; + /** The {@link ModuleProperties} for the system, or null if none exists. */ final ModuleProperties mModuleProperties; @@ -831,7 +834,7 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { } if (!event.recognitionStillActive) { - model.setStopped(); + model.setStoppedLocked(); } try { @@ -918,7 +921,7 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { MetricsLogger.count(mContext, "sth_recognition_aborted", 1); ModelData modelData = getModelDataForLocked(event.soundModelHandle); if (modelData != null && modelData.isModelStarted()) { - modelData.setStopped(); + modelData.setStoppedLocked(); try { modelData.getCallback().onRecognitionPaused(); } catch (DeadObjectException e) { @@ -972,7 +975,7 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { } if (!event.recognitionStillActive) { - modelData.setStopped(); + modelData.setStoppedLocked(); } try { @@ -1200,7 +1203,7 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { if (modelData.isModelStarted()) { Slog.d(TAG, "Stopping previously started dangling model " + modelData.getHandle()); if (mModule.stopRecognition(modelData.getHandle()) == STATUS_OK) { - modelData.setStopped(); + modelData.setStoppedLocked(); modelData.setRequested(false); } else { Slog.e(TAG, "Failed to stop model " + modelData.getHandle()); @@ -1249,7 +1252,7 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { private ModelData getOrCreateGenericModelDataLocked(UUID modelId) { ModelData modelData = mModelDataMap.get(modelId); if (modelData == null) { - modelData = ModelData.createGenericModelData(modelId); + modelData = createGenericModelData(modelId); mModelDataMap.put(modelId, modelData); } else if (!modelData.isGenericModel()) { Slog.e(TAG, "UUID already used for non-generic model."); @@ -1281,7 +1284,7 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { mKeyphraseUuidMap.remove(keyphraseId); mModelDataMap.remove(modelId); mKeyphraseUuidMap.put(keyphraseId, modelId); - ModelData modelData = ModelData.createKeyphraseModelData(modelId); + ModelData modelData = createKeyphraseModelData(modelId); mModelDataMap.put(modelId, modelData); return modelData; } @@ -1413,18 +1416,26 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { Slog.w(TAG, "RemoteException in onError", e); } } - } else { - modelData.setStopped(); - MetricsLogger.count(mContext, "sth_stop_recognition_success", 1); - // Notify of pause if needed. - if (notify) { - try { - callback.onRecognitionPaused(); - } catch (DeadObjectException e) { - forceStopAndUnloadModelLocked(modelData, e); - } catch (RemoteException e) { - Slog.w(TAG, "RemoteException in onRecognitionPaused", e); - } + return status; + } + + // Wait for model to be stopped. + try { + modelData.waitStoppedLocked(STOP_TIMEOUT_MS); + } catch (InterruptedException e) { + Slog.e(TAG, "Didn't receive model stop callback"); + return SoundTrigger.STATUS_ERROR; + } + + MetricsLogger.count(mContext, "sth_stop_recognition_success", 1); + // Notify of pause if needed. + if (notify) { + try { + callback.onRecognitionPaused(); + } catch (DeadObjectException e) { + forceStopAndUnloadModelLocked(modelData, e); + } catch (RemoteException e) { + Slog.w(TAG, "RemoteException in onRecognitionPaused", e); } } if (DBG) { @@ -1459,7 +1470,7 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { // This class encapsulates the callbacks, state, handles and any other information that // represents a model. - private static class ModelData { + private class ModelData { // Model not loaded (and hence not started). static final int MODEL_NOTLOADED = 0; @@ -1516,17 +1527,9 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { mModelType = modelType; } - static ModelData createKeyphraseModelData(UUID modelId) { - return new ModelData(modelId, SoundModel.TYPE_KEYPHRASE); - } - - static ModelData createGenericModelData(UUID modelId) { - return new ModelData(modelId, SoundModel.TYPE_GENERIC_SOUND); - } - // Note that most of the functionality in this Java class will not work for // SoundModel.TYPE_UNKNOWN nevertheless we have it since lower layers support it. - static ModelData createModelDataOfUnknownType(UUID modelId) { + ModelData createModelDataOfUnknownType(UUID modelId) { return new ModelData(modelId, SoundModel.TYPE_UNKNOWN); } @@ -1550,8 +1553,20 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { mModelState = MODEL_STARTED; } - synchronized void setStopped() { + synchronized void setStoppedLocked() { mModelState = MODEL_LOADED; + mLock.notifyAll(); + } + + void waitStoppedLocked(long timeoutMs) throws InterruptedException { + long deadline = System.currentTimeMillis() + timeoutMs; + while (mModelState == MODEL_STARTED) { + long waitTime = deadline - System.currentTimeMillis(); + if (waitTime <= 0) { + throw new InterruptedException(); + } + mLock.wait(waitTime); + } } synchronized void setLoaded() { @@ -1571,6 +1586,7 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { mRecognitionConfig = null; mRequested = false; mCallback = null; + notifyAll(); } synchronized void clearCallback() { @@ -1675,4 +1691,12 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { return "Model type: " + type + "\n"; } } + + ModelData createKeyphraseModelData(UUID modelId) { + return new ModelData(modelId, SoundModel.TYPE_KEYPHRASE); + } + + ModelData createGenericModelData(UUID modelId) { + return new ModelData(modelId, SoundModel.TYPE_GENERIC_SOUND); + } }