Fix modelUnload deadlock

The concurrency policy for this module is that the lock should not be
held while calling stopRecognition and unloadModel into the HAL --
however, that was not being observed for unloadModel.

Update the framework state under the lock, and then drop it to call
unloadModel on the HAL interface. There are no post-conditions on the
STHAL call that the framework cares about, so this we don't need an
unloading state, and this is safe.

There could be an issue if we state protected the onModelUnloaded
callback, but the current implementation fails to do this regardless
(leaves it up to the callers), so it is not an issue.

Fixes: 374908863
Test: compiles
Flag: EXEMPT trivial bugfix
Change-Id: I625af495bb28baf6cdccd1916eda772889e71597

1 files changed, 13 insertions, 13 deletions

diff --git a/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerModule.java b/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerModule.java
index 45a7fafa90a7..07969bd2cd43 100644
--- a/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerModule.java
+++ b/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerModule.java
@@ -304,11 +304,17 @@ class SoundTriggerModule implements IBinder.DeathRecipient, ISoundTriggerHal.Glo
         @Override
         public void unloadModel(int modelHandle) {
             synchronized (SoundTriggerModule.this) {
-                int sessionId;
                 checkValid();
-                sessionId = mLoadedModels.get(modelHandle).unload();
-                mAudioSessionProvider.releaseSession(sessionId);
+                final var session = mLoadedModels.get(modelHandle).getSession();
+                mLoadedModels.remove(modelHandle);
+                mAudioSessionProvider.releaseSession(session.mSessionHandle);
             }
+            // We don't need to post-synchronize on anything once the HAL has finished the unload
+            // and dispatched any appropriate callbacks -- since we don't do any state checking
+            // onModelUnloaded regardless.
+            // This is generally safe since there is no post-condition on the framework side when
+            // a model is unloaded. We assume that we won't ever have a modelHandle collision.
+            mHalService.unloadSoundModel(modelHandle);
         }
 
         @Override
@@ -402,6 +408,10 @@ class SoundTriggerModule implements IBinder.DeathRecipient, ISoundTriggerHal.Glo
                 return mState;
             }
 
+            private SoundTriggerMiddlewareImpl.AudioSessionProvider.AudioSession getSession() {
+                return mSession;
+            }
+
             private void setState(@NonNull ModelState state) {
                 mState = state;
                 SoundTriggerModule.this.notifyAll();
@@ -426,16 +436,6 @@ class SoundTriggerModule implements IBinder.DeathRecipient, ISoundTriggerHal.Glo
                 return mHandle;
             }
 
-            /**
-             * Unloads the model.
-             * @return The audio session handle.
-             */
-            private int unload() {
-                mHalService.unloadSoundModel(mHandle);
-                mLoadedModels.remove(mHandle);
-                return mSession.mSessionHandle;
-            }
-
             private IBinder startRecognition(@NonNull RecognitionConfig config) {
                 if (mIsStopping == true) {
                     throw new RecoverableException(Status.INTERNAL_ERROR, "Race occurred");