Avoid potential failure to boot with bad storage
Discovered by inspection: It is possible in the
event of file system issues to throw an exception
on boot.
This change moves logic previously done in the
PackageStatusStorage constructor to an initialize()
method that can be called on service start up and
handled more gracefully: if there is a storage problem
a warning will be logged and time zone updates
feature will just not be enabled.
Also:
Commit a7d21f8c321ae7149b68625a5c8502abe005ed7b
switched to FileUtils.createDir() but the mkdir()
(implied by createDir()) wasn't removed along with
it but was redundant.
Units tests run with:
atest FrameworksServiceTestsCases:PackageStatusStorageTest \
FrameworksServiceTestsCases:PackageTrackerTest
Test: See above
Test: PTS: run pts -m PtsTimeZoneTestCases
Test: Manual test: deleted /data/system/timezone and rebooted, dir
recreated
Change-Id: I84ab3152d0a9d6781e5665e8615b3ab92b6c7a40
diff --git a/services/core/java/com/android/server/timezone/PackageStatusStorage.java b/services/core/java/com/android/server/timezone/PackageStatusStorage.java
index 5601c91..251a277 100644
--- a/services/core/java/com/android/server/timezone/PackageStatusStorage.java
+++ b/services/core/java/com/android/server/timezone/PackageStatusStorage.java
@@ -82,12 +82,16 @@
PackageStatusStorage(File storageDir) {
mPackageStatusFile = new AtomicFile(new File(storageDir, "package-status.xml"));
+ }
+
+ /**
+ * Initialize any storage, as needed.
+ *
+ * @throws IOException if the storage could not be initialized
+ */
+ void initialize() throws IOException {
if (!mPackageStatusFile.getBaseFile().exists()) {
- try {
- insertInitialPackageStatus();
- } catch (IOException e) {
- throw new IllegalStateException(e);
- }
+ insertInitialPackageStatus();
}
}
@@ -342,13 +346,13 @@
}
/** Only used during tests to force a known table state. */
- public void forceCheckStateForTests(int checkStatus, PackageVersions packageVersions) {
+ public void forceCheckStateForTests(int checkStatus, PackageVersions packageVersions)
+ throws IOException {
synchronized (this) {
try {
- int optimisticLockId = getCurrentOptimisticLockId();
- writePackageStatusWithOptimisticLockCheck(optimisticLockId, optimisticLockId,
- checkStatus, packageVersions);
- } catch (IOException | ParseException e) {
+ final int initialOptimisticLockId = (int) System.currentTimeMillis();
+ writePackageStatusLocked(checkStatus, initialOptimisticLockId, packageVersions);
+ } catch (IOException e) {
throw new IllegalStateException(e);
}
}
diff --git a/services/core/java/com/android/server/timezone/PackageTracker.java b/services/core/java/com/android/server/timezone/PackageTracker.java
index c362c80..0e8d8bc 100644
--- a/services/core/java/com/android/server/timezone/PackageTracker.java
+++ b/services/core/java/com/android/server/timezone/PackageTracker.java
@@ -28,6 +28,7 @@
import android.util.Slog;
import java.io.File;
+import java.io.IOException;
import java.io.PrintWriter;
import java.time.Clock;
@@ -97,10 +98,6 @@
Clock elapsedRealtimeClock = SystemClock.elapsedRealtimeClock();
PackageTrackerHelperImpl helperImpl = new PackageTrackerHelperImpl(context);
File storageDir = FileUtils.createDir(Environment.getDataSystemDirectory(), "timezone");
- if (!storageDir.exists()) {
- storageDir.mkdir();
- }
-
return new PackageTracker(
elapsedRealtimeClock /* elapsedRealtimeClock */,
helperImpl /* configHelper */,
@@ -121,11 +118,11 @@
}
@VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE)
- protected synchronized void start() {
+ protected synchronized boolean start() {
mTrackingEnabled = mConfigHelper.isTrackingEnabled();
if (!mTrackingEnabled) {
Slog.i(TAG, "Time zone updater / data package tracking explicitly disabled.");
- return;
+ return false;
}
mUpdateAppPackageName = mConfigHelper.getUpdateAppPackageName();
@@ -143,6 +140,14 @@
mCheckTriggered = false;
mCheckFailureCount = 0;
+ // Initialize the storage, as needed.
+ try {
+ mPackageStatusStorage.initialize();
+ } catch (IOException e) {
+ Slog.w(TAG, "PackageTracker storage could not be initialized.", e);
+ return false;
+ }
+
// Initialize the intent helper.
mIntentHelper.initialize(mUpdateAppPackageName, mDataAppPackageName, this);
@@ -152,6 +157,7 @@
mIntentHelper.scheduleReliabilityTrigger(mDelayBeforeReliabilityCheckMillis);
Slog.i(TAG, "Time zone updater / data package tracking enabled");
+ return true;
}
/**
diff --git a/services/core/java/com/android/server/timezone/RulesManagerService.java b/services/core/java/com/android/server/timezone/RulesManagerService.java
index 52b49ba..30fc63c 100644
--- a/services/core/java/com/android/server/timezone/RulesManagerService.java
+++ b/services/core/java/com/android/server/timezone/RulesManagerService.java
@@ -121,6 +121,7 @@
}
public void start() {
+ // Return value deliberately ignored: no action required on failure to start.
mPackageTracker.start();
}
diff --git a/services/tests/servicestests/src/com/android/server/timezone/PackageStatusStorageTest.java b/services/tests/servicestests/src/com/android/server/timezone/PackageStatusStorageTest.java
index b57cac0..74013b7 100644
--- a/services/tests/servicestests/src/com/android/server/timezone/PackageStatusStorageTest.java
+++ b/services/tests/servicestests/src/com/android/server/timezone/PackageStatusStorageTest.java
@@ -25,6 +25,7 @@
import android.support.test.filters.SmallTest;
import java.io.File;
+import java.io.IOException;
import java.io.PrintWriter;
import java.io.StringWriter;
@@ -33,6 +34,7 @@
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
+import static org.junit.Assert.fail;
@SmallTest
public class PackageStatusStorageTest {
@@ -48,6 +50,7 @@
// Using the instrumentation context means the database is created in a test app-specific
// directory.
mPackageStatusStorage = new PackageStatusStorage(dataDir);
+ mPackageStatusStorage.initialize();
}
@After
@@ -56,6 +59,16 @@
}
@Test
+ public void initialize_fail() {
+ File readOnlyDir = new File("/system/does/not/exist");
+ PackageStatusStorage packageStatusStorage = new PackageStatusStorage(readOnlyDir);
+ try {
+ packageStatusStorage.initialize();
+ fail();
+ } catch (IOException expected) {}
+ }
+
+ @Test
public void getPackageStatus_initialState() {
assertNull(mPackageStatusStorage.getPackageStatus());
}
diff --git a/services/tests/servicestests/src/com/android/server/timezone/PackageTrackerTest.java b/services/tests/servicestests/src/com/android/server/timezone/PackageTrackerTest.java
index 5d739a3..9cf6392 100644
--- a/services/tests/servicestests/src/com/android/server/timezone/PackageTrackerTest.java
+++ b/services/tests/servicestests/src/com/android/server/timezone/PackageTrackerTest.java
@@ -30,6 +30,8 @@
import android.support.test.InstrumentationRegistry;
import android.support.test.filters.SmallTest;
+import java.io.File;
+import java.io.IOException;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.time.Clock;
@@ -104,7 +106,7 @@
configureTrackingDisabled();
// Initialize the tracker.
- mPackageTracker.start();
+ assertFalse(mPackageTracker.start());
// Check the IntentHelper was not initialized.
mFakeIntentHelper.assertNotInitialized();
@@ -119,7 +121,7 @@
configureTrackingDisabled();
// Initialize the tracker.
- mPackageTracker.start();
+ assertFalse(mPackageTracker.start());
// Check reliability triggering state.
mFakeIntentHelper.assertReliabilityTriggerNotScheduled();
@@ -141,7 +143,7 @@
configureTrackingDisabled();
// Initialize the tracker.
- mPackageTracker.start();
+ assertFalse(mPackageTracker.start());
// Check reliability triggering state.
mFakeIntentHelper.assertReliabilityTriggerNotScheduled();
@@ -166,7 +168,7 @@
mPackageStatusStorage.generateCheckToken(INITIAL_APP_PACKAGE_VERSIONS);
// Initialize the tracker.
- mPackageTracker.start();
+ assertFalse(mPackageTracker.start());
// Check reliability triggering state.
mFakeIntentHelper.assertReliabilityTriggerNotScheduled();
@@ -262,6 +264,35 @@
}
@Test
+ public void trackingEnabled_storageInitializationFails() throws Exception {
+ // Create a PackageStateStorage that will fail to initialize.
+ PackageStatusStorage packageStatusStorage =
+ new PackageStatusStorage(new File("/system/does/not/exist"));
+
+ // Create a new PackageTracker to use the bad storage.
+ mPackageTracker = new PackageTracker(
+ mFakeClock,
+ mMockConfigHelper,
+ mMockPackageManagerHelper,
+ packageStatusStorage,
+ mFakeIntentHelper);
+
+ // Set up device configuration.
+ configureTrackingEnabled();
+ configureReliabilityConfigSettingsOk();
+ configureValidApplications();
+
+ // Initialize the tracker.
+ assertFalse(mPackageTracker.start());
+
+ // Check the IntentHelper was not initialized.
+ mFakeIntentHelper.assertNotInitialized();
+
+ // Check reliability triggering state.
+ mFakeIntentHelper.assertReliabilityTriggerNotScheduled();
+ }
+
+ @Test
public void trackingEnabled_packageUpdate_badUpdateAppManifestEntry() throws Exception {
// Set up device configuration.
configureTrackingEnabled();
@@ -269,7 +300,7 @@
configureValidApplications();
// Initialize the tracker.
- mPackageTracker.start();
+ assertTrue(mPackageTracker.start());
// Check the intent helper is properly configured.
checkIntentHelperInitializedAndReliabilityTrackingEnabled();
@@ -306,7 +337,7 @@
configureValidApplications();
// Initialize the tracker.
- mPackageTracker.start();
+ assertTrue(mPackageTracker.start());
// Check the intent helper is properly configured.
checkIntentHelperInitializedAndReliabilityTrackingEnabled();
@@ -351,7 +382,7 @@
configureValidApplications();
// Initialize the tracker.
- mPackageTracker.start();
+ assertTrue(mPackageTracker.start());
// Check the intent helper is properly configured.
checkIntentHelperInitializedAndReliabilityTrackingEnabled();
@@ -399,7 +430,7 @@
configureValidApplications();
// Initialize the tracker.
- mPackageTracker.start();
+ assertTrue(mPackageTracker.start());
// Check the intent helper is properly configured.
checkIntentHelperInitializedAndReliabilityTrackingEnabled();
@@ -438,7 +469,7 @@
configureValidApplications();
// Initialize the tracker.
- mPackageTracker.start();
+ assertTrue(mPackageTracker.start());
// Check the intent helper is properly configured.
checkIntentHelperInitializedAndReliabilityTrackingEnabled();
@@ -484,7 +515,7 @@
configureValidApplications();
// Initialize the tracker.
- mPackageTracker.start();
+ assertTrue(mPackageTracker.start());
// Check the intent helper is properly configured.
checkIntentHelperInitializedAndReliabilityTrackingEnabled();
@@ -533,7 +564,7 @@
configureValidApplications();
// Initialize the tracker.
- mPackageTracker.start();
+ assertTrue(mPackageTracker.start());
// Check the intent helper is properly configured.
checkIntentHelperInitializedAndReliabilityTrackingEnabled();
@@ -593,7 +624,7 @@
configureValidApplications();
// Initialize the package tracker.
- mPackageTracker.start();
+ assertTrue(mPackageTracker.start());
// Check the intent helper is properly configured.
checkIntentHelperInitializedAndReliabilityTrackingEnabled();
@@ -655,7 +686,7 @@
configureValidApplications();
// Initialize the package tracker.
- mPackageTracker.start();
+ assertTrue(mPackageTracker.start());
// Check the intent helper is properly configured.
checkIntentHelperInitializedAndReliabilityTrackingEnabled();
@@ -711,7 +742,7 @@
configureValidApplications();
// Initialize the package tracker.
- mPackageTracker.start();
+ assertTrue(mPackageTracker.start());
// Check the intent helper is properly configured.
checkIntentHelperInitializedAndReliabilityTrackingEnabled();
@@ -760,7 +791,7 @@
PackageVersions packageVersions = configureValidApplications();
// Initialize the package tracker.
- mPackageTracker.start();
+ assertTrue(mPackageTracker.start());
// Check the intent helper is properly configured.
checkIntentHelperInitializedAndReliabilityTrackingEnabled();
@@ -797,7 +828,7 @@
PackageStatus.CHECK_COMPLETED_SUCCESS, packageVersions);
// Initialize the package tracker.
- mPackageTracker.start();
+ assertTrue(mPackageTracker.start());
// Check the intent helper is properly configured.
checkIntentHelperInitializedAndReliabilityTrackingEnabled();
@@ -837,7 +868,7 @@
PackageStatus.CHECK_COMPLETED_FAILURE, oldPackageVersions);
// Initialize the package tracker.
- mPackageTracker.start();
+ assertTrue(mPackageTracker.start());
// Check the intent helper is properly configured.
checkIntentHelperInitializedAndReliabilityTrackingEnabled();
@@ -883,7 +914,7 @@
PackageStatus.CHECK_COMPLETED_FAILURE, oldPackageVersions);
// Initialize the package tracker.
- mPackageTracker.start();
+ assertTrue(mPackageTracker.start());
// Check the intent helper is properly configured.
checkIntentHelperInitializedAndReliabilityTrackingEnabled();
@@ -942,7 +973,7 @@
PackageStatus.CHECK_COMPLETED_FAILURE, oldPackageVersions);
// Initialize the package tracker.
- mPackageTracker.start();
+ assertTrue(mPackageTracker.start());
// Check the intent helper is properly configured.
checkIntentHelperInitializedAndReliabilityTrackingEnabled();
@@ -1017,7 +1048,7 @@
PackageStatus.CHECK_COMPLETED_FAILURE, oldPackageVersions);
// Initialize the package tracker.
- mPackageTracker.start();
+ assertTrue(mPackageTracker.start());
// Check the intent helper is properly configured.
checkIntentHelperInitializedAndReliabilityTrackingEnabled();
@@ -1083,7 +1114,7 @@
PackageStatus.CHECK_COMPLETED_SUCCESS, currentPackageVersions);
// Initialize the package tracker.
- mPackageTracker.start();
+ assertTrue(mPackageTracker.start());
// Check the intent helper is properly configured.
checkIntentHelperInitializedAndReliabilityTrackingEnabled();
@@ -1138,7 +1169,7 @@
PackageStatus.CHECK_COMPLETED_SUCCESS, currentPackageVersions);
// Initialize the package tracker.
- mPackageTracker.start();
+ assertTrue(mPackageTracker.start());
// Check the intent helper is properly configured.
checkIntentHelperInitializedAndReliabilityTrackingEnabled();