Skip to content

Commit 6367b16

Browse files
jsharkeyAndroid (Google) Code Review
authored andcommitted
Merge "Recover from Throwable in FileRotator, dump." into jb-dev
2 parents 9669a0c + 6de357e commit 6367b16

File tree

4 files changed

+91
-17
lines changed

4 files changed

+91
-17
lines changed

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

Lines changed: 50 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@
1919
import android.os.FileUtils;
2020
import android.util.Slog;
2121

22-
import com.android.internal.util.FileRotator.Rewriter;
23-
2422
import java.io.BufferedInputStream;
2523
import java.io.BufferedOutputStream;
2624
import java.io.File;
@@ -29,8 +27,11 @@
2927
import java.io.IOException;
3028
import java.io.InputStream;
3129
import java.io.OutputStream;
30+
import java.util.zip.ZipEntry;
31+
import java.util.zip.ZipOutputStream;
3232

3333
import libcore.io.IoUtils;
34+
import libcore.io.Streams;
3435

3536
/**
3637
* 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
137138
public void deleteAll() {
138139
final FileInfo info = new FileInfo(mPrefix);
139140
for (String name : mBasePath.list()) {
140-
if (!info.parse(name)) continue;
141+
if (info.parse(name)) {
142+
// delete each file that matches parser
143+
new File(mBasePath, name).delete();
144+
}
145+
}
146+
}
147+
148+
/**
149+
* Dump all files managed by this rotator for debugging purposes.
150+
*/
151+
public void dumpAll(OutputStream os) throws IOException {
152+
final ZipOutputStream zos = new ZipOutputStream(os);
153+
try {
154+
final FileInfo info = new FileInfo(mPrefix);
155+
for (String name : mBasePath.list()) {
156+
if (info.parse(name)) {
157+
final ZipEntry entry = new ZipEntry(name);
158+
zos.putNextEntry(entry);
141159

142-
// delete each file that matches parser
143-
new File(mBasePath, name).delete();
160+
final File file = new File(mBasePath, name);
161+
final FileInputStream is = new FileInputStream(file);
162+
try {
163+
Streams.copy(is, zos);
164+
} finally {
165+
IoUtils.closeQuietly(is);
166+
}
167+
168+
zos.closeEntry();
169+
}
170+
}
171+
} finally {
172+
IoUtils.closeQuietly(zos);
144173
}
145174
}
146175

@@ -159,22 +188,22 @@ public void rewriteActive(Rewriter rewriter, long currentTimeMillis)
159188
public void combineActive(final Reader reader, final Writer writer, long currentTimeMillis)
160189
throws IOException {
161190
rewriteActive(new Rewriter() {
162-
/** {@inheritDoc} */
191+
@Override
163192
public void reset() {
164193
// ignored
165194
}
166195

167-
/** {@inheritDoc} */
196+
@Override
168197
public void read(InputStream in) throws IOException {
169198
reader.read(in);
170199
}
171200

172-
/** {@inheritDoc} */
201+
@Override
173202
public boolean shouldWrite() {
174203
return true;
175204
}
176205

177-
/** {@inheritDoc} */
206+
@Override
178207
public void write(OutputStream out) throws IOException {
179208
writer.write(out);
180209
}
@@ -224,11 +253,11 @@ private void rewriteSingle(Rewriter rewriter, String name) throws IOException {
224253

225254
// write success, delete backup
226255
backupFile.delete();
227-
} catch (IOException e) {
256+
} catch (Throwable t) {
228257
// write failed, delete file and restore backup
229258
file.delete();
230259
backupFile.renameTo(file);
231-
throw e;
260+
throw rethrowAsIoException(t);
232261
}
233262

234263
} else {
@@ -241,11 +270,11 @@ private void rewriteSingle(Rewriter rewriter, String name) throws IOException {
241270

242271
// write success, delete empty backup
243272
backupFile.delete();
244-
} catch (IOException e) {
273+
} catch (Throwable t) {
245274
// write failed, delete file and empty backup
246275
file.delete();
247276
backupFile.delete();
248-
throw e;
277+
throw rethrowAsIoException(t);
249278
}
250279
}
251280
}
@@ -353,6 +382,14 @@ private static void writeFile(File file, Writer writer) throws IOException {
353382
}
354383
}
355384

385+
private static IOException rethrowAsIoException(Throwable t) throws IOException {
386+
if (t instanceof IOException) {
387+
throw (IOException) t;
388+
} else {
389+
throw new IOException(t.getMessage(), t);
390+
}
391+
}
392+
356393
/**
357394
* Details for a rotated file, either parsed from an existing filename, or
358395
* ready to be built into a new filename.

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -187,12 +187,12 @@ public void testThrowRestoresBackup() throws Exception {
187187
rotate.combineActive(reader, new Writer() {
188188
public void write(OutputStream out) throws IOException {
189189
new DataOutputStream(out).writeUTF("bar");
190-
throw new ProtocolException("yikes");
190+
throw new NullPointerException("yikes");
191191
}
192192
}, currentTime);
193193

194194
fail("woah, somehow able to write exception");
195-
} catch (ProtocolException e) {
195+
} catch (IOException e) {
196196
// expected from above
197197
}
198198

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

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import android.net.NetworkStatsHistory;
2727
import android.net.NetworkTemplate;
2828
import android.net.TrafficStats;
29+
import android.os.DropBoxManager;
2930
import android.util.Log;
3031
import android.util.MathUtils;
3132
import android.util.Slog;
@@ -34,6 +35,7 @@
3435
import com.android.internal.util.IndentingPrintWriter;
3536
import com.google.android.collect.Sets;
3637

38+
import java.io.ByteArrayOutputStream;
3739
import java.io.DataOutputStream;
3840
import java.io.File;
3941
import java.io.IOException;
@@ -43,6 +45,8 @@
4345
import java.util.HashSet;
4446
import java.util.Map;
4547

48+
import libcore.io.IoUtils;
49+
4650
/**
4751
* Logic to record deltas between periodic {@link NetworkStats} snapshots into
4852
* {@link NetworkStatsHistory} that belong to {@link NetworkStatsCollection}.
@@ -56,8 +60,14 @@ public class NetworkStatsRecorder {
5660
private static final boolean LOGD = false;
5761
private static final boolean LOGV = false;
5862

63+
private static final String TAG_NETSTATS_DUMP = "netstats_dump";
64+
65+
/** Dump before deleting in {@link #recoverFromWtf()}. */
66+
private static final boolean DUMP_BEFORE_DELETE = true;
67+
5968
private final FileRotator mRotator;
6069
private final NonMonotonicObserver<String> mObserver;
70+
private final DropBoxManager mDropBox;
6171
private final String mCookie;
6272

6373
private final long mBucketDuration;
@@ -74,9 +84,10 @@ public class NetworkStatsRecorder {
7484
private WeakReference<NetworkStatsCollection> mComplete;
7585

7686
public NetworkStatsRecorder(FileRotator rotator, NonMonotonicObserver<String> observer,
77-
String cookie, long bucketDuration, boolean onlyTags) {
87+
DropBoxManager dropBox, String cookie, long bucketDuration, boolean onlyTags) {
7888
mRotator = checkNotNull(rotator, "missing FileRotator");
7989
mObserver = checkNotNull(observer, "missing NonMonotonicObserver");
90+
mDropBox = checkNotNull(dropBox, "missing DropBoxManager");
8091
mCookie = cookie;
8192

8293
mBucketDuration = bucketDuration;
@@ -122,6 +133,7 @@ public NetworkStatsCollection getOrLoadCompleteLocked() {
122133
mComplete = new WeakReference<NetworkStatsCollection>(complete);
123134
} catch (IOException e) {
124135
Log.wtf(TAG, "problem completely reading network stats", e);
136+
recoverFromWtf();
125137
}
126138
}
127139
return complete;
@@ -212,6 +224,7 @@ public void forcePersistLocked(long currentTimeMillis) {
212224
mPending.reset();
213225
} catch (IOException e) {
214226
Log.wtf(TAG, "problem persisting pending stats", e);
227+
recoverFromWtf();
215228
}
216229
}
217230
}
@@ -226,6 +239,7 @@ public void removeUidLocked(int uid) {
226239
mRotator.rewriteAll(new RemoveUidRewriter(mBucketDuration, uid));
227240
} catch (IOException e) {
228241
Log.wtf(TAG, "problem removing UID " + uid, e);
242+
recoverFromWtf();
229243
}
230244

231245
// clear UID from current stats snapshot
@@ -355,4 +369,25 @@ public void dumpLocked(IndentingPrintWriter pw, boolean fullHistory) {
355369
mSinceBoot.dump(pw);
356370
}
357371
}
372+
373+
/**
374+
* Recover from {@link FileRotator} failure by dumping state to
375+
* {@link DropBoxManager} and deleting contents.
376+
*/
377+
private void recoverFromWtf() {
378+
if (DUMP_BEFORE_DELETE) {
379+
final ByteArrayOutputStream os = new ByteArrayOutputStream();
380+
try {
381+
mRotator.dumpAll(os);
382+
} catch (IOException e) {
383+
// ignore partial contents
384+
os.reset();
385+
} finally {
386+
IoUtils.closeQuietly(os);
387+
}
388+
mDropBox.addData(TAG_NETSTATS_DUMP, os.toByteArray(), 0);
389+
}
390+
391+
mRotator.deleteAll();
392+
}
358393
}

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -338,9 +338,11 @@ public void systemReady() {
338338

339339
private NetworkStatsRecorder buildRecorder(
340340
String prefix, NetworkStatsSettings.Config config, boolean includeTags) {
341+
final DropBoxManager dropBox = (DropBoxManager) mContext.getSystemService(
342+
Context.DROPBOX_SERVICE);
341343
return new NetworkStatsRecorder(new FileRotator(
342344
mBaseDir, prefix, config.rotateAgeMillis, config.deleteAgeMillis),
343-
mNonMonotonicObserver, prefix, config.bucketDuration, includeTags);
345+
mNonMonotonicObserver, dropBox, prefix, config.bucketDuration, includeTags);
344346
}
345347

346348
private void shutdownLocked() {

0 commit comments

Comments
 (0)