From 9fac2c86ce2a441463cb523e20746a92e6f735f8 Mon Sep 17 00:00:00 2001 From: Ytai Ben-Tsvi Date: Thu, 9 Apr 2020 15:21:07 -0700 Subject: [PATCH] Reboot the soundtrigger HAL on failure ... and avoid rebooting system_server. There is no benefit in rebooting the system_server. Fixes: 153461865 Test: Tested via a shell command that the sound trigger middleware service can indeed reboot the HAL and correctly recover from it dying. Change-Id: Ie476968b6acf97bf701d0e2929e7bd099e19cd56 --- .../SoundTriggerMiddlewareService.java | 41 ++++++++++++++++++- .../SoundTriggerMiddlewareValidation.java | 8 +--- 2 files changed, 41 insertions(+), 8 deletions(-) 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 929d92f56c443..635cb613b934d 100644 --- a/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareService.java +++ b/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareService.java @@ -27,7 +27,9 @@ 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; @@ -99,6 +101,28 @@ 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; @@ -119,7 +143,6 @@ public class SoundTriggerMiddlewareService extends ISoundTriggerMiddlewareServic @Override public void unloadModel(int modelHandle) throws RemoteException { mDelegate.unloadModel(modelHandle); - ; } @Override @@ -159,6 +182,22 @@ 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; + } + } } /** 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 da1b7f3113c41..23259558eeb79 100644 --- a/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareValidation.java +++ b/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareValidation.java @@ -149,13 +149,7 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware e.getMessage()); } - /* Throwing an exception is not enough in this case. When the HAL behaves unexpectedly, the - system service and the HAL must be reset and the client must be notified. Without a full - reset in this catastrophic case, the state of the HAL and the system service cannot be - guaranteed to the client. - */ - Log.wtf(TAG, "Crashing system server due to unrecoverable exception", e); - Process.killProcess(Process.myPid()); + Log.wtf(TAG, "Unexpected exception", e); throw new InternalServerError(e); }