Skip to content
Commit dd59df09 authored by Tobias Thierer's avatar Tobias Thierer
Browse files

DataChangedJournal.forEach(): Fix fragile loop condition.

Since commit c31a839f, 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 c31a839f (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
parent 19a88b6d
Loading
Loading
Loading
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment