From 627920cbcb5335d2de067a059dcbd552871e153a Mon Sep 17 00:00:00 2001 From: Neil Fuller Date: Thu, 24 Dec 2020 16:40:37 +0000 Subject: [PATCH] Handle provider initialization failure Handle provider initialization failure gracefully. Now the provider is declared "permanently failed" and the thrown exception does not pop out the top of the stack causing a system server crash. Test: Withdrew a required permission for the only registered provider, verified device failed to boot to UI before, and now it does. Bug: 176812518 Change-Id: I541f5f300b7841c1342e0255fd2ba586d6429706 --- .../BinderLocationTimeZoneProvider.java | 5 ---- .../LocationTimeZoneManagerService.java | 6 +++- .../timezone/LocationTimeZoneProvider.java | 29 +++++++++++-------- .../LocationTimeZoneProviderProxy.java | 2 +- .../NullLocationTimeZoneProvider.java | 6 ---- .../location/timezone/ControllerImplTest.java | 5 ---- 6 files changed, 23 insertions(+), 30 deletions(-) diff --git a/services/core/java/com/android/server/location/timezone/BinderLocationTimeZoneProvider.java b/services/core/java/com/android/server/location/timezone/BinderLocationTimeZoneProvider.java index b9c23b73062a9..0881cd2b0324c 100644 --- a/services/core/java/com/android/server/location/timezone/BinderLocationTimeZoneProvider.java +++ b/services/core/java/com/android/server/location/timezone/BinderLocationTimeZoneProvider.java @@ -161,11 +161,6 @@ class BinderLocationTimeZoneProvider extends LocationTimeZoneProvider { mProxy.setRequest(request); } - @Override - void logWarn(String msg) { - Slog.w(TAG, msg); - } - @Override public void dump(@NonNull IndentingPrintWriter ipw, @Nullable String[] args) { synchronized (mSharedLock) { diff --git a/services/core/java/com/android/server/location/timezone/LocationTimeZoneManagerService.java b/services/core/java/com/android/server/location/timezone/LocationTimeZoneManagerService.java index ab64f9728ef48..a23b9d7039d09 100644 --- a/services/core/java/com/android/server/location/timezone/LocationTimeZoneManagerService.java +++ b/services/core/java/com/android/server/location/timezone/LocationTimeZoneManagerService.java @@ -285,8 +285,12 @@ public class LocationTimeZoneManagerService extends Binder { } static void warnLog(String msg) { + warnLog(msg, null); + } + + static void warnLog(String msg, @Nullable Throwable t) { if (Log.isLoggable(TAG, Log.WARN)) { - Slog.w(TAG, msg); + Slog.w(TAG, msg, t); } } } diff --git a/services/core/java/com/android/server/location/timezone/LocationTimeZoneProvider.java b/services/core/java/com/android/server/location/timezone/LocationTimeZoneProvider.java index 8b51ab4522c99..e55d1cc74b7bd 100644 --- a/services/core/java/com/android/server/location/timezone/LocationTimeZoneProvider.java +++ b/services/core/java/com/android/server/location/timezone/LocationTimeZoneProvider.java @@ -17,6 +17,7 @@ package com.android.server.location.timezone; import static com.android.server.location.timezone.LocationTimeZoneManagerService.debugLog; +import static com.android.server.location.timezone.LocationTimeZoneManagerService.warnLog; import static com.android.server.location.timezone.LocationTimeZoneProvider.ProviderState.PROVIDER_STATE_PERM_FAILED; import static com.android.server.location.timezone.LocationTimeZoneProvider.ProviderState.PROVIDER_STATE_STARTED_CERTAIN; import static com.android.server.location.timezone.LocationTimeZoneProvider.ProviderState.PROVIDER_STATE_STARTED_INITIALIZING; @@ -345,12 +346,21 @@ abstract class LocationTimeZoneProvider implements Dumpable { } mProviderListener = Objects.requireNonNull(providerListener); ProviderState currentState = ProviderState.createStartingState(this); - ProviderState newState = currentState.newState( + currentState = currentState.newState( PROVIDER_STATE_STOPPED, null, null, "initialize() called"); - setCurrentState(newState, false); + setCurrentState(currentState, false); - onInitialize(); + // Guard against uncaught exceptions due to initialization problems. + try { + onInitialize(); + } catch (RuntimeException e) { + warnLog("Unable to initialize the provider", e); + currentState = currentState + .newState(PROVIDER_STATE_PERM_FAILED, null, null, + "Provider failed to initialize"); + setCurrentState(currentState, true); + } } } @@ -498,7 +508,7 @@ abstract class LocationTimeZoneProvider implements Dumpable { case PROVIDER_STATE_PERM_FAILED: { // After entering perm failed, there is nothing to do. The remote peer is // supposed to stop sending events after it has reported perm failure. - logWarn("handleTimeZoneProviderEvent: Event=" + timeZoneProviderEvent + warnLog("handleTimeZoneProviderEvent: Event=" + timeZoneProviderEvent + " received for provider=" + this + " when in failed state"); return; } @@ -509,7 +519,7 @@ abstract class LocationTimeZoneProvider implements Dumpable { + " Failure event=" + timeZoneProviderEvent + " received for stopped provider=" + this + ", entering permanently failed state"; - logWarn(msg); + warnLog(msg); ProviderState newState = currentState.newState( PROVIDER_STATE_PERM_FAILED, null, null, msg); setCurrentState(newState, true); @@ -522,7 +532,7 @@ abstract class LocationTimeZoneProvider implements Dumpable { case EVENT_TYPE_UNCERTAIN: { // Any geolocation-related events received for a stopped provider are // ignored: they should not happen. - logWarn("handleTimeZoneProviderEvent:" + warnLog("handleTimeZoneProviderEvent:" + " event=" + timeZoneProviderEvent + " received for stopped provider=" + this + ", ignoring"); @@ -544,7 +554,7 @@ abstract class LocationTimeZoneProvider implements Dumpable { + " Failure event=" + timeZoneProviderEvent + " received for provider=" + this + ", entering permanently failed state"; - logWarn(msg); + warnLog(msg); ProviderState newState = currentState.newState( PROVIDER_STATE_PERM_FAILED, null, null, msg); setCurrentState(newState, true); @@ -584,11 +594,6 @@ abstract class LocationTimeZoneProvider implements Dumpable { } } - /** - * Implemented by subclasses. - */ - abstract void logWarn(String msg); - @GuardedBy("mSharedLock") private void assertIsStarted() { ProviderState currentState = mCurrentState.get(); diff --git a/services/core/java/com/android/server/location/timezone/LocationTimeZoneProviderProxy.java b/services/core/java/com/android/server/location/timezone/LocationTimeZoneProviderProxy.java index 91b52f14d9301..16f9e97bb52e3 100644 --- a/services/core/java/com/android/server/location/timezone/LocationTimeZoneProviderProxy.java +++ b/services/core/java/com/android/server/location/timezone/LocationTimeZoneProviderProxy.java @@ -79,8 +79,8 @@ abstract class LocationTimeZoneProviderProxy implements Dumpable { throw new IllegalStateException("listener already set"); } this.mListener = listener; + onInitialize(); } - onInitialize(); } /** diff --git a/services/core/java/com/android/server/location/timezone/NullLocationTimeZoneProvider.java b/services/core/java/com/android/server/location/timezone/NullLocationTimeZoneProvider.java index e11a7cee43db9..4b321e628e43a 100644 --- a/services/core/java/com/android/server/location/timezone/NullLocationTimeZoneProvider.java +++ b/services/core/java/com/android/server/location/timezone/NullLocationTimeZoneProvider.java @@ -21,7 +21,6 @@ import static com.android.server.location.timezone.LocationTimeZoneProvider.Prov import android.annotation.NonNull; import android.annotation.Nullable; import android.util.IndentingPrintWriter; -import android.util.Slog; import java.time.Duration; @@ -73,11 +72,6 @@ class NullLocationTimeZoneProvider extends LocationTimeZoneProvider { // Ignored - this implementation is always permanently failed. } - @Override - void logWarn(String msg) { - Slog.w(TAG, msg); - } - @Override public void dump(@NonNull IndentingPrintWriter ipw, @Nullable String[] args) { synchronized (mSharedLock) { diff --git a/services/tests/servicestests/src/com/android/server/location/timezone/ControllerImplTest.java b/services/tests/servicestests/src/com/android/server/location/timezone/ControllerImplTest.java index dc8123741b8df..4de4d95004ddf 100644 --- a/services/tests/servicestests/src/com/android/server/location/timezone/ControllerImplTest.java +++ b/services/tests/servicestests/src/com/android/server/location/timezone/ControllerImplTest.java @@ -1052,11 +1052,6 @@ public class ControllerImplTest { // Nothing needed for tests. } - @Override - void logWarn(String msg) { - System.out.println(msg); - } - @Override public void dump(IndentingPrintWriter pw, String[] args) { // Nothing needed for tests.