UsageStatsService: Update file index to prevent double checkin

We seem to have renamed a file as checked-in twice, which means
we checked it in twice and created a malformed name with the suffix
"-c-c" instead of the correct suffix "-c".

Bug:18280677
Change-Id: Ie3164010898a333e5d9b97151d285ea376de799e

diff --git a/services/usage/java/com/android/server/usage/UsageStatsDatabase.java b/services/usage/java/com/android/server/usage/UsageStatsDatabase.java
index 098b3ef..26ced03 100644 (file)
--- a/services/usage/java/com/android/server/usage/UsageStatsDatabase.java
+++ b/services/usage/java/com/android/server/usage/UsageStatsDatabase.java
@@ -40,6 +40,7 @@ class UsageStatsDatabase {
     private static final String TAG = "UsageStatsDatabase";
     private static final boolean DEBUG = UsageStatsService.DEBUG;
     private static final String BAK_SUFFIX = ".bak";
+    private static final String CHECKED_IN_SUFFIX = UsageStatsXml.CHECKED_IN_SUFFIX;
 
     private final Object mLock = new Object();
     private final File[] mIntervalDirs;
@@ -114,14 +115,17 @@ class UsageStatsDatabase {
             final TimeSparseArray<AtomicFile> files =
                     mSortedStatFiles[UsageStatsManager.INTERVAL_DAILY];
             final int fileCount = files.size();
-            int start = 0;
-            while (start < fileCount - 1) {
-                if (!files.valueAt(start).getBaseFile().getName().endsWith("-c")) {
-                    break;
+
+            // We may have holes in the checkin (if there was an error)
+            // so find the last checked-in file and go from there.
+            int lastCheckin = -1;
+            for (int i = 0; i < fileCount - 1; i++) {
+                if (files.valueAt(i).getBaseFile().getPath().endsWith(CHECKED_IN_SUFFIX)) {
+                    lastCheckin = i;
                 }
-                start++;
             }
 
+            final int start = lastCheckin + 1;
             if (start == fileCount - 1) {
                 return true;
             }
@@ -143,8 +147,8 @@ class UsageStatsDatabase {
             // are marked as checked-in.
             for (int i = start; i < fileCount - 1; i++) {
                 final AtomicFile file = files.valueAt(i);
-                final File checkedInFile = new File(file.getBaseFile().getParent(),
-                        file.getBaseFile().getName() + "-c");
+                final File checkedInFile = new File(
+                        file.getBaseFile().getPath() + CHECKED_IN_SUFFIX);
                 if (!file.getBaseFile().renameTo(checkedInFile)) {
                     // We must return success, as we've already marked some files as checked-in.
                     // It's better to repeat ourselves than to lose data.
@@ -152,6 +156,10 @@ class UsageStatsDatabase {
                             + " as checked-in");
                     return true;
                 }
+
+                // AtomicFile needs to set a new backup path with the same -c extension, so
+                // we replace the old AtomicFile with the updated one.
+                files.setValueAt(i, new AtomicFile(checkedInFile));
             }
         }
         return true;
@@ -240,8 +248,13 @@ class UsageStatsDatabase {
                         } catch (IOException e) {
                             // Ignore, this is just to make sure there are no backups.
                         }
-                        final File newFile = new File(file.getBaseFile().getParentFile(),
-                                Long.toString(newTime));
+
+                        String newName = Long.toString(newTime);
+                        if (file.getBaseFile().getName().endsWith(CHECKED_IN_SUFFIX)) {
+                            newName = newName + CHECKED_IN_SUFFIX;
+                        }
+
+                        final File newFile = new File(file.getBaseFile().getParentFile(), newName);
                         Slog.i(TAG, "Moving file " + file.getBaseFile().getAbsolutePath() + " to "
                                 + newFile.getAbsolutePath());
                         file.getBaseFile().renameTo(newFile);

diff --git a/services/usage/java/com/android/server/usage/UsageStatsXml.java b/services/usage/java/com/android/server/usage/UsageStatsXml.java
index 26148ce..186813e 100644 (file)
--- a/services/usage/java/com/android/server/usage/UsageStatsXml.java
+++ b/services/usage/java/com/android/server/usage/UsageStatsXml.java
@@ -31,17 +31,21 @@ public class UsageStatsXml {
     private static final int CURRENT_VERSION = 1;
     private static final String USAGESTATS_TAG = "usagestats";
     private static final String VERSION_ATTR = "version";
-    private static final String CHECKED_IN_SUFFIX = "-c";
+    static final String CHECKED_IN_SUFFIX = "-c";
 
     public static long parseBeginTime(AtomicFile file) {
         return parseBeginTime(file.getBaseFile());
     }
 
     public static long parseBeginTime(File file) {
-        final String name = file.getName();
-        if (name.endsWith(CHECKED_IN_SUFFIX)) {
-            return Long.parseLong(
-                    name.substring(0, name.length() - CHECKED_IN_SUFFIX.length()));
+        String name = file.getName();
+
+        // Eat as many occurrences of -c as possible. This is due to a bug where -c
+        // would be appended more than once to a checked-in file, causing a crash
+        // on boot when indexing files since Long.parseLong() will puke on anything but
+        // a number.
+        while (name.endsWith(CHECKED_IN_SUFFIX)) {
+            name = name.substring(0, name.length() - CHECKED_IN_SUFFIX.length());
         }
         return Long.parseLong(name);
     }