From e867da399e2adaeb4ff9ef175fa2007cfe79141e Mon Sep 17 00:00:00 2001
From: Ytai Ben-Tsvi <ytai@google.com>
Date: Tue, 26 Jan 2021 12:11:40 -0800
Subject: Better fix for race condition in SoundTriggerHw2Enforcer

This reverts commit 139f7c23898bff49ea6eb9a1f6cdb7e43d28d8ac.

That commit had a potential for a deadlock, since calls both to and
from the HAL are synchronized and because of the requirement that
events are not permitted after stop() has been called, the HAL implementation
must block stop() for any outstanding events, but those events might
be blocked during the callback.

This fix avoids the original issue addressed by that commit in a
different manner: it simply reversed the ordering of setting the model
state and invoking start(), so that if an event arrives before start()
returns, we would still consider it valid.

Fixes: 177795410
Test: Manual verification of STHAL operation.
      Sent patch to bug reporter to apply and test.
Change-Id: I5b08158eaed8e4bba9f77249b7d5e5b73c33a744
(cherry picked from commit 9efec8111bdd68ef8bc3ed4053ac1c4b69192a9b)
---
 .../SoundTriggerHw2Enforcer.java                   | 34 ++++++++++++----------
 1 file changed, 18 insertions(+), 16 deletions(-)

(limited to 'services')

diff --git a/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerHw2Enforcer.java b/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerHw2Enforcer.java
index 90ac69a97158..cf7460b306cd 100644
--- a/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerHw2Enforcer.java
+++ b/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerHw2Enforcer.java
@@ -42,7 +42,7 @@ public class SoundTriggerHw2Enforcer implements ISoundTriggerHw2 {
     static final String TAG = "SoundTriggerHw2Enforcer";
 
     final ISoundTriggerHw2 mUnderlying;
-    final Map<Integer, Boolean> mModelStates = new HashMap<>();
+    Map<Integer, Boolean> mModelStates = new HashMap<>();
 
     public SoundTriggerHw2Enforcer(
             ISoundTriggerHw2 underlying) {
@@ -62,12 +62,12 @@ public class SoundTriggerHw2Enforcer implements ISoundTriggerHw2 {
     public int loadSoundModel(ISoundTriggerHw.SoundModel soundModel, Callback callback,
             int cookie) {
         try {
+            int handle = mUnderlying.loadSoundModel(soundModel, new CallbackEnforcer(callback),
+                    cookie);
             synchronized (mModelStates) {
-                int handle = mUnderlying.loadSoundModel(soundModel, new CallbackEnforcer(callback),
-                        cookie);
                 mModelStates.put(handle, false);
-                return handle;
             }
+            return handle;
         } catch (RuntimeException e) {
             throw handleException(e);
         }
@@ -77,13 +77,13 @@ public class SoundTriggerHw2Enforcer implements ISoundTriggerHw2 {
     public int loadPhraseSoundModel(ISoundTriggerHw.PhraseSoundModel soundModel, Callback callback,
             int cookie) {
         try {
+            int handle = mUnderlying.loadPhraseSoundModel(soundModel,
+                    new CallbackEnforcer(callback),
+                    cookie);
             synchronized (mModelStates) {
-                int handle = mUnderlying.loadPhraseSoundModel(soundModel,
-                        new CallbackEnforcer(callback),
-                        cookie);
                 mModelStates.put(handle, false);
-                return handle;
             }
+            return handle;
         } catch (RuntimeException e) {
             throw handleException(e);
         }
@@ -92,8 +92,8 @@ public class SoundTriggerHw2Enforcer implements ISoundTriggerHw2 {
     @Override
     public void unloadSoundModel(int modelHandle) {
         try {
+            mUnderlying.unloadSoundModel(modelHandle);
             synchronized (mModelStates) {
-                mUnderlying.unloadSoundModel(modelHandle);
                 mModelStates.remove(modelHandle);
             }
         } catch (RuntimeException e) {
@@ -104,8 +104,8 @@ public class SoundTriggerHw2Enforcer implements ISoundTriggerHw2 {
     @Override
     public void stopRecognition(int modelHandle) {
         try {
+            mUnderlying.stopRecognition(modelHandle);
             synchronized (mModelStates) {
-                mUnderlying.stopRecognition(modelHandle);
                 mModelStates.replace(modelHandle, false);
             }
         } catch (RuntimeException e) {
@@ -116,8 +116,8 @@ public class SoundTriggerHw2Enforcer implements ISoundTriggerHw2 {
     @Override
     public void stopAllRecognitions() {
         try {
+            mUnderlying.stopAllRecognitions();
             synchronized (mModelStates) {
-                mUnderlying.stopAllRecognitions();
                 for (Map.Entry<Integer, Boolean> entry : mModelStates.entrySet()) {
                     entry.setValue(false);
                 }
@@ -130,12 +130,14 @@ public class SoundTriggerHw2Enforcer implements ISoundTriggerHw2 {
     @Override
     public void startRecognition(int modelHandle, RecognitionConfig config, Callback callback,
             int cookie) {
+        // It is possible that an event will be sent before the HAL returns from the
+        // startRecognition call, thus it is important to set the state to active before the call.
+        synchronized (mModelStates) {
+            mModelStates.replace(modelHandle, true);
+        }
         try {
-            synchronized (mModelStates) {
-                mUnderlying.startRecognition(modelHandle, config, new CallbackEnforcer(callback),
-                        cookie);
-                mModelStates.replace(modelHandle, true);
-            }
+            mUnderlying.startRecognition(modelHandle, config, new CallbackEnforcer(callback),
+                    cookie);
         } catch (RuntimeException e) {
             throw handleException(e);
         }
-- 
cgit v1.2.3-59-g8ed1b