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
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 45a7faf..07969bd 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 @@
@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 @@
return mState;
}
+ private SoundTriggerMiddlewareImpl.AudioSessionProvider.AudioSession getSession() {
+ return mSession;
+ }
+
private void setState(@NonNull ModelState state) {
mState = state;
SoundTriggerModule.this.notifyAll();
@@ -426,16 +436,6 @@
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");