Maybe fix issue #2457218: Corrupt batterystats.bin file preventing phone boot - LIBtt68127

No steps to repro, but makes the code more robust by using the standard
JournaledFile class and doing sanity checks on the input it reads.

This required moving the JournaledFile class in to the framework (and
we really should get rid of either it or AtomicFile, but they have
different recovery semantics so that is tough).  Also went through and
cleaned up the file management in various places.

Change-Id: Ieb7268d8435e77dff66b6e67bb63b62e5dea572e

7 files changed, 179 insertions, 53 deletions

diff --git a/core/java/android/app/ContextImpl.java b/core/java/android/app/ContextImpl.java
index 4464ab9..f296f43 100644
--- a/core/java/android/app/ContextImpl.java
+++ b/core/java/android/app/ContextImpl.java
@@ -2935,9 +2935,14 @@ class ContextImpl extends Context {
         private boolean writeFileLocked() {
             // Rename the current file so it may be used as a backup during the next read
             if (mFile.exists()) {
-                if (!mFile.renameTo(mBackupFile)) {
-                    Log.e(TAG, "Couldn't rename file " + mFile + " to backup file " + mBackupFile);
-                    return false;
+                if (!mBackupFile.exists()) {
+                    if (!mFile.renameTo(mBackupFile)) {
+                        Log.e(TAG, "Couldn't rename file " + mFile
+                                + " to backup file " + mBackupFile);
+                        return false;
+                    }
+                } else {
+                    mFile.delete();
                 }
             }
             
diff --git a/core/java/android/os/FileUtils.java b/core/java/android/os/FileUtils.java
index 4780cf3..a17b7fe 100644
--- a/core/java/android/os/FileUtils.java
+++ b/core/java/android/os/FileUtils.java
@@ -115,6 +115,9 @@ public class FileUtils
      */
     public static boolean copyToFile(InputStream inputStream, File destFile) {
         try {
+            if (destFile.exists()) {
+                destFile.delete();
+            }
             OutputStream out = new FileOutputStream(destFile);
             try {
                 byte[] buffer = new byte[4096];
diff --git a/core/java/android/view/ViewDebug.java b/core/java/android/view/ViewDebug.java
index 8311bdc..2b5489c 100644
--- a/core/java/android/view/ViewDebug.java
+++ b/core/java/android/view/ViewDebug.java
@@ -546,6 +546,9 @@ public class ViewDebug {
         recyclerDump = new File(Environment.getExternalStorageDirectory(), "view-recycler/");
         recyclerDump = new File(recyclerDump, sRecyclerTracePrefix + ".traces");
         try {
+            if (recyclerDump.exists()) {
+                recyclerDump.delete();
+            }
             final FileOutputStream file = new FileOutputStream(recyclerDump);
             final DataOutputStream out = new DataOutputStream(file);
 
diff --git a/core/java/com/android/internal/backup/LocalTransport.java b/core/java/com/android/internal/backup/LocalTransport.java
index 9e4b606..b436363 100644
--- a/core/java/com/android/internal/backup/LocalTransport.java
+++ b/core/java/com/android/internal/backup/LocalTransport.java
@@ -105,6 +105,9 @@ public class LocalTransport extends IBackupTransport.Stub {
                         + " key64=" + base64Key);
 
                 if (dataSize >= 0) {
+                    if (entityFile.exists()) {
+                        entityFile.delete();
+                    }
                     FileOutputStream entity = new FileOutputStream(entityFile);
 
                     if (dataSize > bufSize) {
diff --git a/core/java/com/android/internal/os/AtomicFile.java b/core/java/com/android/internal/os/AtomicFile.java
index ca0345f..e675ef0 100644
--- a/core/java/com/android/internal/os/AtomicFile.java
+++ b/core/java/com/android/internal/os/AtomicFile.java
@@ -45,12 +45,13 @@ public class AtomicFile {
     public FileOutputStream startWrite() throws IOException {
         // Rename the current file so it may be used as a backup during the next read
         if (mBaseName.exists()) {
-            if (!mBaseName.renameTo(mBackupName)) {
-                mBackupName.delete();
+            if (!mBackupName.exists()) {
                 if (!mBaseName.renameTo(mBackupName)) {
                     Log.w("AtomicFile", "Couldn't rename file " + mBaseName
                             + " to backup file " + mBackupName);
                 }
+            } else {
+                mBaseName.delete();
             }
         }
         FileOutputStream str = null;
diff --git a/core/java/com/android/internal/os/BatteryStatsImpl.java b/core/java/com/android/internal/os/BatteryStatsImpl.java
index 71ccb3b..46769ce 100644
--- a/core/java/com/android/internal/os/BatteryStatsImpl.java
+++ b/core/java/com/android/internal/os/BatteryStatsImpl.java
@@ -16,6 +16,8 @@
 
 package com.android.internal.os;
 
+import com.android.internal.util.JournaledFile;
+
 import android.bluetooth.BluetoothHeadset;
 import android.net.TrafficStats;
 import android.os.BatteryStats;
@@ -30,6 +32,7 @@ import android.telephony.TelephonyManager;
 import android.util.Log;
 import android.util.PrintWriterPrinter;
 import android.util.Printer;
+import android.util.Slog;
 import android.util.SparseArray;
 
 import java.io.BufferedReader;
@@ -57,7 +60,7 @@ public final class BatteryStatsImpl extends BatteryStats {
     private static final int MAGIC = 0xBA757475; // 'BATSTATS' 
 
     // Current on-disk Parcel version
-    private static final int VERSION = 42;
+    private static final int VERSION = 43;
 
     // The maximum number of names wakelocks we will keep track of
     // per uid; once the limit is reached, we batch the remaining wakelocks
@@ -68,8 +71,7 @@ public final class BatteryStatsImpl extends BatteryStats {
     
     private static int sNumSpeedSteps;
 
-    private final File mFile;
-    private final File mBackupFile;
+    private final JournaledFile mFile;
 
     /**
      * The statistics we have collected organized by uids.
@@ -216,7 +218,7 @@ public final class BatteryStatsImpl extends BatteryStats {
     
     // For debugging
     public BatteryStatsImpl() {
-        mFile = mBackupFile = null;
+        mFile = null;
     }
 
     public static interface Unpluggable {
@@ -2704,8 +2706,7 @@ public final class BatteryStatsImpl extends BatteryStats {
     }
 
     public BatteryStatsImpl(String filename) {
-        mFile = new File(filename);
-        mBackupFile = new File(filename + ".bak");
+        mFile = new JournaledFile(new File(filename), new File(filename + ".tmp"));
         mStartCount++;
         mScreenOnTimer = new StopwatchTimer(-1, null, mUnpluggables);
         for (int i=0; i<NUM_SCREEN_BRIGHTNESS_BINS; i++) {
@@ -2736,7 +2737,7 @@ public final class BatteryStatsImpl extends BatteryStats {
     }
 
     public BatteryStatsImpl(Parcel p) {
-        mFile = mBackupFile = null;
+        mFile = null;
         readFromParcel(p);
     }
 
@@ -2799,7 +2800,7 @@ public final class BatteryStatsImpl extends BatteryStats {
         
         if (m == null) {
             // Not crashing might make board bringup easier.
-            Log.w(TAG, "Couldn't get kernel wake lock stats");
+            Slog.w(TAG, "Couldn't get kernel wake lock stats");
             return;
         }
 
@@ -3047,26 +3048,19 @@ public final class BatteryStatsImpl extends BatteryStats {
         return u.getServiceStatsLocked(pkg, name);
     }
 
+    private static JournaledFile makeJournaledFile() {
+        final String base = "/data/system/device_policies.xml";
+        return new JournaledFile(new File(base), new File(base + ".tmp"));
+    }
+
     public void writeLocked() {
-        if ((mFile == null) || (mBackupFile == null)) {
-            Log.w("BatteryStats", "writeLocked: no file associated with this instance");
+        if (mFile == null) {
+            Slog.w("BatteryStats", "writeLocked: no file associated with this instance");
             return;
         }
 
-        // Keep the old file around until we know the new one has
-        // been successfully written.
-        if (mFile.exists()) {
-            if (mBackupFile.exists()) {
-                mBackupFile.delete();
-            }
-            if (!mFile.renameTo(mBackupFile)) {
-                Log.w("BatteryStats", "Failed to back up file before writing new stats");
-                return;
-            }
-        }
-
         try {
-            FileOutputStream stream = new FileOutputStream(mFile);
+            FileOutputStream stream = new FileOutputStream(mFile.chooseForWrite());
             Parcel out = Parcel.obtain();
             writeSummaryToParcel(out);
             stream.write(out.marshall());
@@ -3074,18 +3068,14 @@ public final class BatteryStatsImpl extends BatteryStats {
 
             stream.flush();
             stream.close();
-            mBackupFile.delete();
+            mFile.commit();
 
             mLastWriteTime = SystemClock.elapsedRealtime();
             return;
         } catch (IOException e) {
-            Log.w("BatteryStats", "Error writing battery statistics", e);
-        }
-        if (mFile.exists()) {
-            if (!mFile.delete()) {
-                Log.w(TAG, "Failed to delete mangled file " + mFile);
-            }
+            Slog.w("BatteryStats", "Error writing battery statistics", e);
         }
+        mFile.rollback();
     }
 
     static byte[] readFully(FileInputStream stream) throws java.io.IOException {
@@ -3112,29 +3102,19 @@ public final class BatteryStatsImpl extends BatteryStats {
     }
 
     public void readLocked() {
-        if ((mFile == null) || (mBackupFile == null)) {
-            Log.w("BatteryStats", "readLocked: no file associated with this instance");
+        if (mFile == null) {
+            Slog.w("BatteryStats", "readLocked: no file associated with this instance");
             return;
         }
 
         mUidStats.clear();
 
-        FileInputStream stream = null;
-        if (mBackupFile.exists()) {
-            try {
-                stream = new FileInputStream(mBackupFile);
-            } catch (java.io.IOException e) {
-                // We'll try for the normal settings file.
-            }
-        }
-
         try {
-            if (stream == null) {
-                if (!mFile.exists()) {
-                    return;
-                }
-                stream = new FileInputStream(mFile);
+            File file = mFile.chooseForRead();
+            if (!file.exists()) {
+                return;
             }
+            FileInputStream stream = new FileInputStream(file);
 
             byte[] raw = readFully(stream);
             Parcel in = Parcel.obtain();
@@ -3144,7 +3124,7 @@ public final class BatteryStatsImpl extends BatteryStats {
 
             readSummaryFromParcel(in);
         } catch(java.io.IOException e) {
-            Log.e("BatteryStats", "Error reading battery statistics", e);
+            Slog.e("BatteryStats", "Error reading battery statistics", e);
         }
     }
 
@@ -3155,7 +3135,7 @@ public final class BatteryStatsImpl extends BatteryStats {
     private void readSummaryFromParcel(Parcel in) {
         final int version = in.readInt();
         if (version != VERSION) {
-            Log.w("BatteryStats", "readFromParcel: version got " + version
+            Slog.w("BatteryStats", "readFromParcel: version got " + version
                 + ", expected " + VERSION + "; erasing old stats");
             return;
         }
@@ -3197,6 +3177,10 @@ public final class BatteryStatsImpl extends BatteryStats {
         mBluetoothOnTimer.readSummaryFromParcelLocked(in);
 
         int NKW = in.readInt();
+        if (NKW > 10000) {
+            Slog.w(TAG, "File corrupt: too many kernel wake locks " + NKW);
+            return;
+        }
         for (int ikw = 0; ikw < NKW; ikw++) {
             if (in.readInt() != 0) {
                 String kwltName = in.readString();
@@ -3207,6 +3191,10 @@ public final class BatteryStatsImpl extends BatteryStats {
         sNumSpeedSteps = in.readInt();
 
         final int NU = in.readInt();
+        if (NU > 10000) {
+            Slog.w(TAG, "File corrupt: too many uids " + NU);
+            return;
+        }
         for (int iu = 0; iu < NU; iu++) {
             int uid = in.readInt();
             Uid u = new Uid(uid);
@@ -3235,6 +3223,10 @@ public final class BatteryStatsImpl extends BatteryStats {
             }
             
             int NW = in.readInt();
+            if (NW > 10000) {
+                Slog.w(TAG, "File corrupt: too many wake locks " + NW);
+                return;
+            }
             for (int iw = 0; iw < NW; iw++) {
                 String wlName = in.readString();
                 if (in.readInt() != 0) {
@@ -3249,6 +3241,10 @@ public final class BatteryStatsImpl extends BatteryStats {
             }
 
             int NP = in.readInt();
+            if (NP > 10000) {
+                Slog.w(TAG, "File corrupt: too many sensors " + NP);
+                return;
+            }
             for (int is = 0; is < NP; is++) {
                 int seNumber = in.readInt();
                 if (in.readInt() != 0) {
@@ -3258,6 +3254,10 @@ public final class BatteryStatsImpl extends BatteryStats {
             }
 
             NP = in.readInt();
+            if (NP > 10000) {
+                Slog.w(TAG, "File corrupt: too many processes " + NP);
+                return;
+            }
             for (int ip = 0; ip < NP; ip++) {
                 String procName = in.readString();
                 Uid.Proc p = u.getProcessStatsLocked(procName);
@@ -3270,6 +3270,10 @@ public final class BatteryStatsImpl extends BatteryStats {
             }
 
             NP = in.readInt();
+            if (NP > 10000) {
+                Slog.w(TAG, "File corrupt: too many packages " + NP);
+                return;
+            }
             for (int ip = 0; ip < NP; ip++) {
                 String pkgName = in.readString();
                 Uid.Pkg p = u.getPackageStatsLocked(pkgName);
diff --git a/core/java/com/android/internal/util/JournaledFile.java b/core/java/com/android/internal/util/JournaledFile.java
new file mode 100644
index 0000000..af0c6c6
--- /dev/null
+++ b/core/java/com/android/internal/util/JournaledFile.java
@@ -0,0 +1,107 @@
+/*
+ * Copyright (C) 2009 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.internal.util;
+
+import java.io.File;
+import java.io.IOException;
+
+public class JournaledFile {
+    File mReal;
+    File mTemp;
+    boolean mWriting;
+
+    public JournaledFile(File real, File temp) {
+        mReal = real;
+        mTemp = temp;
+    }
+
+    /** Returns the file for you to read.
+     * @more
+     * Prefers the real file.  If it doesn't exist, uses the temp one, and then copies
+     * it to the real one.  If there is both a real file and a temp one, assumes that the
+     * temp one isn't fully written and deletes it.
+     */
+    public File chooseForRead() {
+        File result;
+        if (mReal.exists()) {
+            result = mReal;
+            if (mTemp.exists()) {
+                mTemp.delete();
+            }
+        } else if (mTemp.exists()) {
+            result = mTemp;
+            mTemp.renameTo(mReal);
+        } else {
+            return mReal;
+        }
+        return result;
+    }
+
+    /**
+     * Returns a file for you to write.
+     * @more
+     * If a write is already happening, throws.  In other words, you must provide your
+     * own locking.
+     * <p>
+     * Call {@link #commit} to commit the changes, or {@link #rollback} to forget the changes.
+     */
+    public File chooseForWrite() {
+        if (mWriting) {
+            throw new IllegalStateException("uncommitted write already in progress");
+        }
+        if (!mReal.exists()) {
+            // If the real one doesn't exist, it's either because this is the first time
+            // or because something went wrong while copying them.  In this case, we can't
+            // trust anything that's in temp.  In order to have the chooseForRead code not
+            // use the temporary one until it's fully written, create an empty file
+            // for real, which will we'll shortly delete.
+            try {
+                mReal.createNewFile();
+            } catch (IOException e) {
+                // Ignore
+            }
+        }
+
+        if (mTemp.exists()) {
+            mTemp.delete();
+        }
+        mWriting = true;
+        return mTemp;
+    }
+
+    /**
+     * Commit changes.
+     */
+    public void commit() {
+        if (!mWriting) {
+            throw new IllegalStateException("no file to commit");
+        }
+        mWriting = false;
+        mTemp.renameTo(mReal);
+    }
+
+    /**
+     * Roll back changes.
+     */
+    public void rollback() {
+        if (!mWriting) {
+            throw new IllegalStateException("no file to roll back");
+        }
+        mWriting = false;
+        mTemp.delete();
+    }
+}