DataChangedJournal.forEach(): Fix fragile loop condition.
Since commit c31a839fd3ecc91807d735884d09fcbaf62e9244, this
logic looped "while (dataInputStream.available() > 0)", which
indicates whether more bytes can be read _without blocking_.
It's possible for this to be false even when there are more
bytes available in the file, which means that this loop was
prone to premature termination; somewhat surprisingly to me, a
DataInputStream(BufferedInputStream(FileInputStream))) wrapper
does return available() > 0 initially (empirically tested on
the latest development version), but it's not clear whether
this will reliably remain the case later on in the file and
how often in practice this failed. Either way, this code
was fragile and subject to potential silent breakage in future
where some packageNames would not have been reliably read.
This CL changes the logic back to reading unconditionally but
catching EOFException, like the corresponding logic did prior
to commit c31a839fd3ecc91807d735884d09fcbaf62e9244 (July 2017).
To demonstrate the original bug through a regression test, I
would have needed to extract a static helper method
@VisibleForTesting forEach(Consumer, InputStream) to demonstrate
the behavior with an InputStream whose available() returns 0.
This didn't seem worth it, so I've only added a comment explaining
the logic to minimize the chance of future regression.
This CL also fixes incorrect use of a try-with-resources statement
that would have not closed the FileInputStream if the
BufferedInputStream ctor had thrown (would not have occurred in
practice).
Bug: 162160897
Test: atest \
FrameworksServicesTests:com.android.server.backup.DataChangedJournalTest
Change-Id: I7641cf9ea269ef050ceb716c4e7fe34166bc8295
diff --git a/services/backup/java/com/android/server/backup/DataChangedJournal.java b/services/backup/java/com/android/server/backup/DataChangedJournal.java
index c8a9855..0e7fc93 100644
--- a/services/backup/java/com/android/server/backup/DataChangedJournal.java
+++ b/services/backup/java/com/android/server/backup/DataChangedJournal.java
@@ -22,9 +22,11 @@
import java.io.BufferedInputStream;
import java.io.DataInputStream;
+import java.io.EOFException;
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
+import java.io.InputStream;
import java.io.RandomAccessFile;
import java.util.ArrayList;
import java.util.List;
@@ -78,15 +80,17 @@
*/
public void forEach(Consumer<String> consumer) throws IOException {
try (
- BufferedInputStream bufferedInputStream = new BufferedInputStream(
- new FileInputStream(mFile), BUFFER_SIZE_BYTES);
- DataInputStream dataInputStream = new DataInputStream(bufferedInputStream)
+ InputStream in = new FileInputStream(mFile);
+ InputStream bufferedIn = new BufferedInputStream(in, BUFFER_SIZE_BYTES);
+ DataInputStream dataInputStream = new DataInputStream(bufferedIn)
) {
- while (dataInputStream.available() > 0) {
+ while (true) {
String packageName = dataInputStream.readUTF();
consumer.accept(packageName);
}
- }
+ } catch (EOFException tolerated) {
+ // no more data; we're done
+ } // other kinds of IOExceptions are error conditions and handled in the caller
}
/**