From 443459b5e9f86353506a0855cf10f6e8c0c1b746 Mon Sep 17 00:00:00 2001 From: Neil Fuller Date: Sun, 10 Jan 2021 17:57:23 +0000 Subject: [PATCH] Fix behavior when primary fails initialization Fix behavior when primary fails initialization: before this, the primary would fail and the controller would attempt to start the secondary, which hadn't been initialized yet. This commit also adds tests to confirm primary, secondary or both provider failure work as intended. Bug: 176812518 Test: atest services/tests/servicestests/src/com/android/server/location/timezone/ControllerImplTest.java Change-Id: I9d14b98f0b2ebb262f4dd8ee1ec82d944a5b57a8 --- .../location/timezone/ControllerImpl.java | 15 ++++ .../location/timezone/ControllerImplTest.java | 83 +++++++++++++++++++ 2 files changed, 98 insertions(+) diff --git a/services/core/java/com/android/server/location/timezone/ControllerImpl.java b/services/core/java/com/android/server/location/timezone/ControllerImpl.java index 03ce8ecd1cc08..9a583d8fa333a 100644 --- a/services/core/java/com/android/server/location/timezone/ControllerImpl.java +++ b/services/core/java/com/android/server/location/timezone/ControllerImpl.java @@ -73,6 +73,10 @@ class ControllerImpl extends LocationTimeZoneProviderController { // Non-null after initialize() private Callback mCallback; + /** Indicates both providers have completed initialization. */ + @GuardedBy("mSharedLock") + private boolean mProvidersInitialized; + /** * Used for scheduling uncertainty timeouts, i.e after a provider has reported uncertainty. * This timeout is not provider-specific: it is started when the controller becomes uncertain @@ -108,6 +112,7 @@ class ControllerImpl extends LocationTimeZoneProviderController { ControllerImpl.this::onProviderStateChange; mPrimaryProvider.initialize(providerListener); mSecondaryProvider.initialize(providerListener); + mProvidersInitialized = true; alterProvidersStartedStateIfRequired( null /* oldConfiguration */, mCurrentUserConfiguration); @@ -322,6 +327,16 @@ class ControllerImpl extends LocationTimeZoneProviderController { assertProviderKnown(provider); synchronized (mSharedLock) { + // Ignore provider state changes during initialization. e.g. if the primary provider + // moves to PROVIDER_STATE_PERM_FAILED during initialization, the secondary will not + // be ready to take over yet. + if (!mProvidersInitialized) { + warnLog("onProviderStateChange: Ignoring provider state change because both" + + " providers have not yet completed initialization." + + " providerState=" + providerState); + return; + } + switch (providerState.stateEnum) { case PROVIDER_STATE_STARTED_INITIALIZING: case PROVIDER_STATE_STOPPED: 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 23365f70b2f42..9ce06a994891d 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 @@ -87,6 +87,81 @@ public class ControllerImplTest { new TestLocationTimeZoneProvider(mTestThreadingDomain, "secondary"); } + @Test + public void initializationFailure_primary() { + ControllerImpl controllerImpl = new ControllerImpl(mTestThreadingDomain, + mTestPrimaryLocationTimeZoneProvider, mTestSecondaryLocationTimeZoneProvider); + TestEnvironment testEnvironment = new TestEnvironment( + mTestThreadingDomain, controllerImpl, USER1_CONFIG_GEO_DETECTION_ENABLED); + Duration expectedInitTimeout = testEnvironment.getProviderInitializationTimeout() + .plus(testEnvironment.getProviderInitializationTimeoutFuzz()); + + mTestPrimaryLocationTimeZoneProvider.setFailDuringInitialization(true); + + // Initialize. After initialization the providers must be initialized and one should be + // started. + controllerImpl.initialize(testEnvironment, mTestCallback); + + mTestPrimaryLocationTimeZoneProvider.assertInitialized(); + mTestSecondaryLocationTimeZoneProvider.assertInitialized(); + + mTestPrimaryLocationTimeZoneProvider.assertIsPermFailedAndCommit(); + mTestSecondaryLocationTimeZoneProvider.assertStateEnumAndConfigAndCommit( + PROVIDER_STATE_STARTED_INITIALIZING, USER1_CONFIG_GEO_DETECTION_ENABLED); + mTestSecondaryLocationTimeZoneProvider.assertInitializationTimeoutSet(expectedInitTimeout); + mTestCallback.assertNoSuggestionMade(); + assertFalse(controllerImpl.isUncertaintyTimeoutSet()); + } + + @Test + public void initializationFailure_secondary() { + ControllerImpl controllerImpl = new ControllerImpl(mTestThreadingDomain, + mTestPrimaryLocationTimeZoneProvider, mTestSecondaryLocationTimeZoneProvider); + TestEnvironment testEnvironment = new TestEnvironment( + mTestThreadingDomain, controllerImpl, USER1_CONFIG_GEO_DETECTION_ENABLED); + Duration expectedInitTimeout = testEnvironment.getProviderInitializationTimeout() + .plus(testEnvironment.getProviderInitializationTimeoutFuzz()); + + mTestSecondaryLocationTimeZoneProvider.setFailDuringInitialization(true); + + // Initialize. After initialization the providers must be initialized and one should be + // started. + controllerImpl.initialize(testEnvironment, mTestCallback); + + mTestPrimaryLocationTimeZoneProvider.assertInitialized(); + mTestSecondaryLocationTimeZoneProvider.assertInitialized(); + + mTestPrimaryLocationTimeZoneProvider.assertStateEnumAndConfigAndCommit( + PROVIDER_STATE_STARTED_INITIALIZING, USER1_CONFIG_GEO_DETECTION_ENABLED); + mTestPrimaryLocationTimeZoneProvider.assertInitializationTimeoutSet(expectedInitTimeout); + mTestSecondaryLocationTimeZoneProvider.assertIsPermFailedAndCommit(); + mTestCallback.assertNoSuggestionMade(); + assertFalse(controllerImpl.isUncertaintyTimeoutSet()); + } + + @Test + public void initializationFailure_both() { + ControllerImpl controllerImpl = new ControllerImpl(mTestThreadingDomain, + mTestPrimaryLocationTimeZoneProvider, mTestSecondaryLocationTimeZoneProvider); + TestEnvironment testEnvironment = new TestEnvironment( + mTestThreadingDomain, controllerImpl, USER1_CONFIG_GEO_DETECTION_ENABLED); + + mTestPrimaryLocationTimeZoneProvider.setFailDuringInitialization(true); + mTestSecondaryLocationTimeZoneProvider.setFailDuringInitialization(true); + + // Initialize. After initialization the providers must be initialized and one should be + // started. + controllerImpl.initialize(testEnvironment, mTestCallback); + + mTestPrimaryLocationTimeZoneProvider.assertInitialized(); + mTestSecondaryLocationTimeZoneProvider.assertInitialized(); + + mTestPrimaryLocationTimeZoneProvider.assertIsPermFailedAndCommit(); + mTestSecondaryLocationTimeZoneProvider.assertIsPermFailedAndCommit(); + mTestCallback.assertUncertainSuggestionMadeAndCommit(); + assertFalse(controllerImpl.isUncertaintyTimeoutSet()); + } + @Test public void initialState_started() { ControllerImpl controllerImpl = new ControllerImpl(mTestThreadingDomain, @@ -1028,6 +1103,7 @@ public class ControllerImplTest { /** Used to track historic provider states for tests. */ private final TestState mTestProviderState = new TestState<>(); + private boolean mFailDuringInitialization; private boolean mInitialized; private boolean mDestroyed; @@ -1038,9 +1114,16 @@ public class ControllerImplTest { super(threadingDomain, providerName); } + public void setFailDuringInitialization(boolean failInitialization) { + mFailDuringInitialization = failInitialization; + } + @Override void onInitialize() { mInitialized = true; + if (mFailDuringInitialization) { + throw new RuntimeException("Simulated initialization failure"); + } } @Override