From 7b6bb40e6a1c0ce385e446467a972e6fad05262c Mon Sep 17 00:00:00 2001 From: Ytai Ben-Tsvi Date: Thu, 26 Mar 2020 14:58:51 -0700 Subject: [PATCH] Gracefully handle module death in SoundTriggerMiddlewareValidation Previously, when the sound trigger module died (typically, when the HAL process crashed), the SoundTriggerMiddlewareValidation module would consider the client's attempt to detach() invalid, since it has not unloaded the models. However, in this case, it is valid to detach without unloading them. Furthermore, dumpsys would not reflect the fact that the session is dead. This is now fixed as well. Fixes: 152523828 Bug: 152470531 Test: Manually killing audio server after sound models are loaded and inspection of the logs and dumpsys. Change-Id: I96c6bcddb25a67ff440c0375b20036d57e5cd837 --- .../SoundTriggerMiddlewareValidation.java | 53 ++++++++++++------- 1 file changed, 33 insertions(+), 20 deletions(-) diff --git a/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareValidation.java b/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareValidation.java index a9e8d3fc7a3a2..06f2d65c13b29 100644 --- a/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareValidation.java +++ b/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareValidation.java @@ -108,6 +108,12 @@ import java.util.Set; public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddlewareService, Dumpable { private static final String TAG = "SoundTriggerMiddlewareValidation"; + private enum ModuleState { + ALIVE, + DETACHED, + DEAD + }; + private final @NonNull ISoundTriggerMiddlewareService mDelegate; private final @NonNull Context mContext; private Map> mModules; @@ -381,6 +387,7 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware private ISoundTriggerModule mDelegate; private @NonNull Map mLoadedModels = new HashMap<>(); private final int mHandle; + private ModuleState mState = ModuleState.ALIVE; ModuleService(int handle, @NonNull ISoundTriggerCallback callback) { mCallback = callback; @@ -406,7 +413,7 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware synchronized (SoundTriggerMiddlewareValidation.this) { // State validation. - if (mDelegate == null) { + if (mState == ModuleState.DETACHED) { throw new IllegalStateException("Module has been detached."); } @@ -430,7 +437,7 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware synchronized (SoundTriggerMiddlewareValidation.this) { // State validation. - if (mDelegate == null) { + if (mState == ModuleState.DETACHED) { throw new IllegalStateException("Module has been detached."); } @@ -453,7 +460,7 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware synchronized (SoundTriggerMiddlewareValidation.this) { // State validation. - if (mDelegate == null) { + if (mState == ModuleState.DETACHED) { throw new IllegalStateException("Module has been detached."); } ModelState modelState = mLoadedModels.get( @@ -486,7 +493,7 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware synchronized (SoundTriggerMiddlewareValidation.this) { // State validation. - if (mDelegate == null) { + if (mState == ModuleState.DETACHED) { throw new IllegalStateException("Module has been detached."); } ModelState modelState = mLoadedModels.get( @@ -520,7 +527,7 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware synchronized (SoundTriggerMiddlewareValidation.this) { // State validation. - if (mDelegate == null) { + if (mState == ModuleState.DETACHED) { throw new IllegalStateException("Module has been detached."); } ModelState modelState = mLoadedModels.get( @@ -549,7 +556,7 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware synchronized (SoundTriggerMiddlewareValidation.this) { // State validation. - if (mDelegate == null) { + if (mState == ModuleState.DETACHED) { throw new IllegalStateException("Module has been detached."); } ModelState modelState = mLoadedModels.get( @@ -577,7 +584,7 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware synchronized (SoundTriggerMiddlewareValidation.this) { // State validation. - if (mDelegate == null) { + if (mState == ModuleState.DETACHED) { throw new IllegalStateException("Module has been detached."); } ModelState modelState = mLoadedModels.get( @@ -605,7 +612,7 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware synchronized (SoundTriggerMiddlewareValidation.this) { // State validation. - if (mDelegate == null) { + if (mState == ModuleState.DETACHED) { throw new IllegalStateException("Module has been detached."); } ModelState modelState = mLoadedModels.get( @@ -634,7 +641,7 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware synchronized (SoundTriggerMiddlewareValidation.this) { // State validation. - if (mDelegate == null) { + if (mState == ModuleState.DETACHED) { throw new IllegalStateException("Module has been detached."); } ModelState modelState = mLoadedModels.get( @@ -663,10 +670,10 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware synchronized (SoundTriggerMiddlewareValidation.this) { // State validation. - if (mDelegate == null) { + if (mState == ModuleState.DETACHED) { throw new IllegalStateException("Module has already been detached."); } - if (!mLoadedModels.isEmpty()) { + if (mState == ModuleState.ALIVE && !mLoadedModels.isEmpty()) { throw new IllegalStateException("Cannot detach while models are loaded."); } @@ -682,13 +689,13 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware // Override toString() in order to have the delegate's ID in it. @Override public String toString() { - return mDelegate.toString(); + return Objects.toString(mDelegate.toString()); } private void detachInternal() { try { mDelegate.detach(); - mDelegate = null; + mState = ModuleState.DETACHED; mCallback.asBinder().unlinkToDeath(this, 0); mModules.get(mHandle).remove(this); } catch (RemoteException e) { @@ -697,13 +704,18 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware } void dump(PrintWriter pw) { - pw.printf("Loaded models for session %s (handle, active)", toString()); - pw.println(); - pw.println("-------------------------------"); - for (Map.Entry entry : mLoadedModels.entrySet()) { - pw.print(entry.getKey()); - pw.print('\t'); - pw.print(entry.getValue().activityState.name()); + if (mState == ModuleState.ALIVE) { + pw.printf("Loaded models for session %s (handle, active)", toString()); + pw.println(); + pw.println("-------------------------------"); + for (Map.Entry entry : mLoadedModels.entrySet()) { + pw.print(entry.getKey()); + pw.print('\t'); + pw.print(entry.getValue().activityState.name()); + pw.println(); + } + } else { + pw.printf("Session %s is dead", toString()); pw.println(); } } @@ -762,6 +774,7 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware public void onModuleDied() { synchronized (SoundTriggerMiddlewareValidation.this) { try { + mState = ModuleState.DEAD; mCallback.onModuleDied(); } catch (RemoteException e) { // Dead client will be handled by binderDied() - no need to handle here.