From 2e9d4a5bcb23269a77952fbd8e3d243866ad6dfd Mon Sep 17 00:00:00 2001 From: Ytai Ben-Tsvi Date: Mon, 20 Apr 2020 11:54:48 -0700 Subject: [PATCH] Reboot sound trigger HAL on exception The previous mechanism for doing that was broken, relied on onTransact() to be called for every method, when in fact it doesn't get called whenever the caller is in the same process as the server. This change moves this behavior to a lower layer, as part of HAL enforcement. Fixes: 153461865 Test: Injected spurious errors to some HAL operations and verified the HAL gets rebooted and recovery is correct according to diagnostics and testing behavior after the reboot. Change-Id: Ic399bf8808cb35fda5b1766cbe30c2ce5c792c2c --- .../SoundTriggerHw2Enforcer.java | 117 ++++++++++++++---- .../SoundTriggerMiddlewareService.java | 40 ------ 2 files changed, 90 insertions(+), 67 deletions(-) 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 a18b690f08cd2..d98ad74a9d6b6 100644 --- a/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerHw2Enforcer.java +++ b/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerHw2Enforcer.java @@ -23,6 +23,7 @@ import android.hardware.soundtrigger.V2_3.Properties; import android.hardware.soundtrigger.V2_3.RecognitionConfig; import android.os.IHwBinder; import android.os.RemoteException; +import android.os.SystemProperties; import android.util.Log; import java.util.HashMap; @@ -48,83 +49,130 @@ public class SoundTriggerHw2Enforcer implements ISoundTriggerHw2 { @Override public Properties getProperties() { - return mUnderlying.getProperties(); + try { + return mUnderlying.getProperties(); + } catch (RuntimeException e) { + throw handleException(e); + } } @Override public int loadSoundModel(ISoundTriggerHw.SoundModel soundModel, Callback callback, int cookie) { - int handle = mUnderlying.loadSoundModel(soundModel, new CallbackEnforcer(callback), cookie); - synchronized (mModelStates) { - mModelStates.put(handle, false); + try { + int handle = mUnderlying.loadSoundModel(soundModel, new CallbackEnforcer(callback), + cookie); + synchronized (mModelStates) { + mModelStates.put(handle, false); + } + return handle; + } catch (RuntimeException e) { + throw handleException(e); } - return handle; } @Override public int loadPhraseSoundModel(ISoundTriggerHw.PhraseSoundModel soundModel, Callback callback, int cookie) { - int handle = mUnderlying.loadPhraseSoundModel(soundModel, new CallbackEnforcer(callback), - cookie); - synchronized (mModelStates) { - mModelStates.put(handle, false); + try { + int handle = mUnderlying.loadPhraseSoundModel(soundModel, + new CallbackEnforcer(callback), + cookie); + synchronized (mModelStates) { + mModelStates.put(handle, false); + } + return handle; + } catch (RuntimeException e) { + throw handleException(e); } - return handle; } @Override public void unloadSoundModel(int modelHandle) { - mUnderlying.unloadSoundModel(modelHandle); - synchronized (mModelStates) { - mModelStates.remove(modelHandle); + try { + mUnderlying.unloadSoundModel(modelHandle); + synchronized (mModelStates) { + mModelStates.remove(modelHandle); + } + } catch (RuntimeException e) { + throw handleException(e); } } @Override public void stopRecognition(int modelHandle) { - mUnderlying.stopRecognition(modelHandle); - synchronized (mModelStates) { - mModelStates.replace(modelHandle, false); + try { + mUnderlying.stopRecognition(modelHandle); + synchronized (mModelStates) { + mModelStates.replace(modelHandle, false); + } + } catch (RuntimeException e) { + throw handleException(e); } } @Override public void stopAllRecognitions() { - mUnderlying.stopAllRecognitions(); - synchronized (mModelStates) { - for (Map.Entry entry : mModelStates.entrySet()) { - entry.setValue(false); + try { + mUnderlying.stopAllRecognitions(); + synchronized (mModelStates) { + for (Map.Entry entry : mModelStates.entrySet()) { + entry.setValue(false); + } } + } catch (RuntimeException e) { + throw handleException(e); } } @Override public void startRecognition(int modelHandle, RecognitionConfig config, Callback callback, int cookie) { - mUnderlying.startRecognition(modelHandle, config, new CallbackEnforcer(callback), cookie); - synchronized (mModelStates) { - mModelStates.replace(modelHandle, true); + try { + mUnderlying.startRecognition(modelHandle, config, new CallbackEnforcer(callback), + cookie); + synchronized (mModelStates) { + mModelStates.replace(modelHandle, true); + } + } catch (RuntimeException e) { + throw handleException(e); } } @Override public void getModelState(int modelHandle) { - mUnderlying.getModelState(modelHandle); + try { + mUnderlying.getModelState(modelHandle); + } catch (RuntimeException e) { + throw handleException(e); + } } @Override public int getModelParameter(int modelHandle, int param) { - return mUnderlying.getModelParameter(modelHandle, param); + try { + return mUnderlying.getModelParameter(modelHandle, param); + } catch (RuntimeException e) { + throw handleException(e); + } } @Override public void setModelParameter(int modelHandle, int param, int value) { - mUnderlying.setModelParameter(modelHandle, param, value); + try { + mUnderlying.setModelParameter(modelHandle, param, value); + } catch (RuntimeException e) { + throw handleException(e); + } } @Override public ModelParameterRange queryParameter(int modelHandle, int param) { - return mUnderlying.queryParameter(modelHandle, param); + try { + return mUnderlying.queryParameter(modelHandle, param); + } catch (RuntimeException e) { + throw handleException(e); + } } @Override @@ -142,6 +190,17 @@ public class SoundTriggerHw2Enforcer implements ISoundTriggerHw2 { return mUnderlying.interfaceDescriptor(); } + private static RuntimeException handleException(RuntimeException e) { + Log.e(TAG, "Exception caught from HAL, rebooting HAL"); + rebootHal(); + throw e; + } + + private static void rebootHal() { + // This property needs to be defined in an init.rc script and trigger a HAL reboot. + SystemProperties.set("sys.audio.restart.hal", "1"); + } + private class CallbackEnforcer implements Callback { private final Callback mUnderlying; @@ -157,6 +216,8 @@ public class SoundTriggerHw2Enforcer implements ISoundTriggerHw2 { synchronized (mModelStates) { if (!mModelStates.getOrDefault(model, false)) { Log.wtfStack(TAG, "Unexpected recognition event for model: " + model); + rebootHal(); + return; } if (event.header.status != android.media.soundtrigger_middleware.RecognitionStatus.FORCED) { @@ -173,6 +234,8 @@ public class SoundTriggerHw2Enforcer implements ISoundTriggerHw2 { synchronized (mModelStates) { if (!mModelStates.getOrDefault(model, false)) { Log.wtfStack(TAG, "Unexpected recognition event for model: " + model); + rebootHal(); + return; } if (event.common.header.status != android.media.soundtrigger_middleware.RecognitionStatus.FORCED) { diff --git a/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareService.java b/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareService.java index 635cb613b934d..4b464d2d3e044 100644 --- a/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareService.java +++ b/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareService.java @@ -27,9 +27,7 @@ import android.media.soundtrigger_middleware.PhraseSoundModel; import android.media.soundtrigger_middleware.RecognitionConfig; import android.media.soundtrigger_middleware.SoundModel; import android.media.soundtrigger_middleware.SoundTriggerModuleDescriptor; -import android.os.Parcel; import android.os.RemoteException; -import android.os.SystemProperties; import android.util.Log; import com.android.server.SystemService; @@ -101,28 +99,6 @@ public class SoundTriggerMiddlewareService extends ISoundTriggerMiddlewareServic } } - @Override - public boolean onTransact(int code, Parcel data, Parcel reply, int flags) - throws RemoteException { - try { - return super.onTransact(code, data, reply, flags); - } catch (InternalServerError e) { - if (e.getCause() instanceof HalException) { - // We recover from any sort of HAL failure by rebooting the HAL process. - // This will likely reboot more than just the sound trigger HAL. - // The rest of the system should be able to tolerate that. - rebootHal(); - } - throw e; - } - } - - private static void rebootHal() { - Log.i(TAG, "Rebooting the sound trigger HAL"); - // This property needs to be defined in an init.rc script and trigger a HAL reboot. - SystemProperties.set("sys.audio.restart.hal", "1"); - } - private final static class ModuleService extends ISoundTriggerModule.Stub { private final ISoundTriggerModule mDelegate; @@ -182,22 +158,6 @@ public class SoundTriggerMiddlewareService extends ISoundTriggerMiddlewareServic public void detach() throws RemoteException { mDelegate.detach(); } - - @Override - public boolean onTransact(int code, Parcel data, Parcel reply, int flags) - throws RemoteException { - try { - return super.onTransact(code, data, reply, flags); - } catch (InternalServerError e) { - if (e.getCause() instanceof HalException) { - // We recover from any sort of HAL failure by rebooting the HAL process. - // This will likely reboot more than just the sound trigger HAL. - // The rest of the system should be able to tolerate that. - rebootHal(); - } - throw e; - } - } } /**