OSDN Git Service

Maybe fix issue #2457218: Corrupt batterystats.bin file preventing phone boot - LIBtt...
authorDianne Hackborn <hackbod@google.com>
Fri, 19 Mar 2010 05:47:17 +0000 (22:47 -0700)
committerDianne Hackborn <hackbod@google.com>
Fri, 19 Mar 2010 20:59:07 +0000 (13:59 -0700)
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

core/java/android/app/ContextImpl.java
core/java/android/os/FileUtils.java
core/java/android/view/ViewDebug.java
core/java/com/android/internal/backup/LocalTransport.java
core/java/com/android/internal/os/AtomicFile.java
core/java/com/android/internal/os/BatteryStatsImpl.java
core/java/com/android/internal/util/JournaledFile.java [moved from services/java/com/android/server/JournaledFile.java with 98% similarity]
services/java/com/android/server/DevicePolicyManagerService.java
services/java/com/android/server/PackageManagerService.java
services/java/com/android/server/WallpaperManagerService.java
services/java/com/android/server/am/UsageStatsService.java

index 4464ab9..f296f43 100644 (file)
@@ -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();
                 }
             }
             
index 4780cf3..a17b7fe 100644 (file)
@@ -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];
index 8311bdc..2b5489c 100644 (file)
@@ -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);
 
index 9e4b606..b436363 100644 (file)
@@ -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) {
index ca0345f..e675ef0 100644 (file)
@@ -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;
index 71ccb3b..46769ce 100644 (file)
@@ -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);
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-package com.android.server;
+package com.android.internal.util;
 
 import java.io.File;
 import java.io.IOException;
index a555244..1d71577 100644 (file)
@@ -18,6 +18,7 @@ package com.android.server;
 
 import com.android.internal.content.PackageMonitor;
 import com.android.internal.util.FastXmlSerializer;
+import com.android.internal.util.JournaledFile;
 import com.android.internal.util.XmlUtils;
 import com.android.internal.widget.LockPatternUtils;
 
index 677ff4a..48b3fbb 100644 (file)
@@ -20,8 +20,8 @@ import com.android.internal.app.IMediaContainerService;
 import com.android.internal.app.ResolverActivity;
 import com.android.internal.content.PackageHelper;
 import com.android.internal.util.FastXmlSerializer;
+import com.android.internal.util.JournaledFile;
 import com.android.internal.util.XmlUtils;
-import com.android.server.JournaledFile;
 
 import org.xmlpull.v1.XmlPullParser;
 import org.xmlpull.v1.XmlPullParserException;
@@ -8280,6 +8280,7 @@ class PackageManagerService extends IPackageManager.Stub {
                         return;
                     }
                 } else {
+                    mSettingsFilename.delete();
                     Slog.w(TAG, "Preserving older settings backup");
                 }
             }
index f4bdd1f..124da4e 100644 (file)
@@ -68,6 +68,7 @@ import org.xmlpull.v1.XmlSerializer;
 import com.android.internal.content.PackageMonitor;
 import com.android.internal.service.wallpaper.ImageWallpaper;
 import com.android.internal.util.FastXmlSerializer;
+import com.android.internal.util.JournaledFile;
 import com.android.server.DevicePolicyManagerService.ActiveAdmin;
 import com.android.server.DevicePolicyManagerService.MyPackageMonitor;
 
@@ -804,6 +805,9 @@ class WallpaperManagerService extends IWallpaperManager.Stub {
                     }
 
                     res = r.openRawResource(resId);
+                    if (WALLPAPER_FILE.exists()) {
+                        WALLPAPER_FILE.delete();
+                    }
                     fos = new FileOutputStream(WALLPAPER_FILE);
 
                     byte[] buffer = new byte[32768];
index d170b02..1b9e1c7 100644 (file)
@@ -383,9 +383,13 @@ public final class UsageStatsService extends IUsageStats.Stub {
             File backupFile = null;
             if (mFile != null && mFile.exists()) {
                 backupFile = new File(mFile.getPath() + ".bak");
-                if (!mFile.renameTo(backupFile)) {
-                    Slog.w(TAG, "Failed to persist new stats");
-                    return;
+                if (!backupFile.exists()) {
+                    if (!mFile.renameTo(backupFile)) {
+                        Slog.w(TAG, "Failed to persist new stats");
+                        return;
+                    }
+                } else {
+                    mFile.delete();
                 }
             }