Recover from Throwable in FileRotator, dump.

In rewriteSingle(), catch Throwable to rollback to backup file,
instead of just IOException. Also add dumpAll() to pack up contents
for later debugging, and use it when encountering bad stats.

Bug: 6467868
Change-Id: Ic8e287cf5a235706811a304a88d71d11d3a79cd4

Author: jsharkey

core/java/com/android/internal/util/FileRotator.java

@@ -19,8 +19,6 @@
 import android.os.FileUtils;
 import android.util.Slog;
 
-import com.android.internal.util.FileRotator.Rewriter;
-
 import java.io.BufferedInputStream;
 import java.io.BufferedOutputStream;
 import java.io.File;
@@ -29,8 +27,11 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipOutputStream;
 
 import libcore.io.IoUtils;
+import libcore.io.Streams;
 
 /**
  * Utility that rotates files over time, similar to {@code logrotate}. There is
@@ -137,10 +138,38 @@ public FileRotator(File basePath, String prefix, long rotateAgeMillis, long dele
     public void deleteAll() {
         final FileInfo info = new FileInfo(mPrefix);
         for (String name : mBasePath.list()) {
-            if (!info.parse(name)) continue;
+            if (info.parse(name)) {
+                // delete each file that matches parser
+                new File(mBasePath, name).delete();
+            }
+        }
+    }
+
+    /**
+     * Dump all files managed by this rotator for debugging purposes.
+     */
+    public void dumpAll(OutputStream os) throws IOException {
+        final ZipOutputStream zos = new ZipOutputStream(os);
+        try {
+            final FileInfo info = new FileInfo(mPrefix);
+            for (String name : mBasePath.list()) {
+                if (info.parse(name)) {
+                    final ZipEntry entry = new ZipEntry(name);
+                    zos.putNextEntry(entry);
 
-            // delete each file that matches parser
-            new File(mBasePath, name).delete();
+                    final File file = new File(mBasePath, name);
+                    final FileInputStream is = new FileInputStream(file);
+                    try {
+                        Streams.copy(is, zos);
+                    } finally {
+                        IoUtils.closeQuietly(is);
+                    }
+
+                    zos.closeEntry();
+                }
+            }
+        } finally {
+            IoUtils.closeQuietly(zos);
         }
     }
 
@@ -159,22 +188,22 @@ public void rewriteActive(Rewriter rewriter, long currentTimeMillis)
     public void combineActive(final Reader reader, final Writer writer, long currentTimeMillis)
             throws IOException {
         rewriteActive(new Rewriter() {
-            /** {@inheritDoc} */
+            @Override
             public void reset() {
                 // ignored
             }
 
-            /** {@inheritDoc} */
+            @Override
             public void read(InputStream in) throws IOException {
                 reader.read(in);
             }
 
-            /** {@inheritDoc} */
+            @Override
             public boolean shouldWrite() {
                 return true;
             }
 
-            /** {@inheritDoc} */
+            @Override
             public void write(OutputStream out) throws IOException {
                 writer.write(out);
             }
@@ -224,11 +253,11 @@ private void rewriteSingle(Rewriter rewriter, String name) throws IOException {
 
                 // write success, delete backup
                 backupFile.delete();
-            } catch (IOException e) {
+            } catch (Throwable t) {
                 // write failed, delete file and restore backup
                 file.delete();
                 backupFile.renameTo(file);
-                throw e;
+                throw rethrowAsIoException(t);
             }
 
         } else {
@@ -241,11 +270,11 @@ private void rewriteSingle(Rewriter rewriter, String name) throws IOException {
 
                 // write success, delete empty backup
                 backupFile.delete();
-            } catch (IOException e) {
+            } catch (Throwable t) {
                 // write failed, delete file and empty backup
                 file.delete();
                 backupFile.delete();
-                throw e;
+                throw rethrowAsIoException(t);
             }
         }
     }
@@ -353,6 +382,14 @@ private static void writeFile(File file, Writer writer) throws IOException {
         }
     }
 
+    private static IOException rethrowAsIoException(Throwable t) throws IOException {
+        if (t instanceof IOException) {
+            throw (IOException) t;
+        } else {
+            throw new IOException(t.getMessage(), t);
+        }
+    }
+
     /**
      * Details for a rotated file, either parsed from an existing filename, or
      * ready to be built into a new filename.

core/tests/coretests/src/com/android/internal/util/FileRotatorTest.java

@@ -187,12 +187,12 @@ public void testThrowRestoresBackup() throws Exception {
             rotate.combineActive(reader, new Writer() {
                 public void write(OutputStream out) throws IOException {
                     new DataOutputStream(out).writeUTF("bar");
-                    throw new ProtocolException("yikes");
+                    throw new NullPointerException("yikes");
                 }
             }, currentTime);
 
             fail("woah, somehow able to write exception");
-        } catch (ProtocolException e) {
+        } catch (IOException e) {
             // expected from above
         }
 

services/java/com/android/server/net/NetworkStatsRecorder.java

@@ -26,6 +26,7 @@
 import android.net.NetworkStatsHistory;
 import android.net.NetworkTemplate;
 import android.net.TrafficStats;
+import android.os.DropBoxManager;
 import android.util.Log;
 import android.util.MathUtils;
 import android.util.Slog;
@@ -34,6 +35,7 @@
 import com.android.internal.util.IndentingPrintWriter;
 import com.google.android.collect.Sets;
 
+import java.io.ByteArrayOutputStream;
 import java.io.DataOutputStream;
 import java.io.File;
 import java.io.IOException;
@@ -43,6 +45,8 @@
 import java.util.HashSet;
 import java.util.Map;
 
+import libcore.io.IoUtils;
+
 /**
  * Logic to record deltas between periodic {@link NetworkStats} snapshots into
  * {@link NetworkStatsHistory} that belong to {@link NetworkStatsCollection}.
@@ -56,8 +60,14 @@ public class NetworkStatsRecorder {
     private static final boolean LOGD = false;
     private static final boolean LOGV = false;
 
+    private static final String TAG_NETSTATS_DUMP = "netstats_dump";
+
+    /** Dump before deleting in {@link #recoverFromWtf()}. */
+    private static final boolean DUMP_BEFORE_DELETE = true;
+
     private final FileRotator mRotator;
     private final NonMonotonicObserver<String> mObserver;
+    private final DropBoxManager mDropBox;
     private final String mCookie;
 
     private final long mBucketDuration;
@@ -74,9 +84,10 @@ public class NetworkStatsRecorder {
     private WeakReference<NetworkStatsCollection> mComplete;
 
     public NetworkStatsRecorder(FileRotator rotator, NonMonotonicObserver<String> observer,
-            String cookie, long bucketDuration, boolean onlyTags) {
+            DropBoxManager dropBox, String cookie, long bucketDuration, boolean onlyTags) {
         mRotator = checkNotNull(rotator, "missing FileRotator");
         mObserver = checkNotNull(observer, "missing NonMonotonicObserver");
+        mDropBox = checkNotNull(dropBox, "missing DropBoxManager");
         mCookie = cookie;
 
         mBucketDuration = bucketDuration;
@@ -122,6 +133,7 @@ public NetworkStatsCollection getOrLoadCompleteLocked() {
                 mComplete = new WeakReference<NetworkStatsCollection>(complete);
             } catch (IOException e) {
                 Log.wtf(TAG, "problem completely reading network stats", e);
+                recoverFromWtf();
             }
         }
         return complete;
@@ -212,6 +224,7 @@ public void forcePersistLocked(long currentTimeMillis) {
                 mPending.reset();
             } catch (IOException e) {
                 Log.wtf(TAG, "problem persisting pending stats", e);
+                recoverFromWtf();
             }
         }
     }
@@ -226,6 +239,7 @@ public void removeUidLocked(int uid) {
             mRotator.rewriteAll(new RemoveUidRewriter(mBucketDuration, uid));
         } catch (IOException e) {
             Log.wtf(TAG, "problem removing UID " + uid, e);
+            recoverFromWtf();
         }
 
         // clear UID from current stats snapshot
@@ -355,4 +369,25 @@ public void dumpLocked(IndentingPrintWriter pw, boolean fullHistory) {
             mSinceBoot.dump(pw);
         }
     }
+
+    /**
+     * Recover from {@link FileRotator} failure by dumping state to
+     * {@link DropBoxManager} and deleting contents.
+     */
+    private void recoverFromWtf() {
+        if (DUMP_BEFORE_DELETE) {
+            final ByteArrayOutputStream os = new ByteArrayOutputStream();
+            try {
+                mRotator.dumpAll(os);
+            } catch (IOException e) {
+                // ignore partial contents
+                os.reset();
+            } finally {
+                IoUtils.closeQuietly(os);
+            }
+            mDropBox.addData(TAG_NETSTATS_DUMP, os.toByteArray(), 0);
+        }
+
+        mRotator.deleteAll();
+    }
 }

services/java/com/android/server/net/NetworkStatsService.java

@@ -338,9 +338,11 @@ public void systemReady() {
 
     private NetworkStatsRecorder buildRecorder(
             String prefix, NetworkStatsSettings.Config config, boolean includeTags) {
+        final DropBoxManager dropBox = (DropBoxManager) mContext.getSystemService(
+                Context.DROPBOX_SERVICE);
         return new NetworkStatsRecorder(new FileRotator(
                 mBaseDir, prefix, config.rotateAgeMillis, config.deleteAgeMillis),
-                mNonMonotonicObserver, prefix, config.bucketDuration, includeTags);
+                mNonMonotonicObserver, dropBox, prefix, config.bucketDuration, includeTags);
     }
 
     private void shutdownLocked() {