Skip to content

Commit

Permalink
Avoid using InputStream.available() to detect EOF while reading del…
Browse files Browse the repository at this point in the history
…imited protos.

It's not a reliable method to check for EOF: it returns how many bytes are guaranteed to be read without blocking, and the base implementation always returns 0.

Instead, leverage the fact that parseDelimitedFrom() returns null if the stream is at EOF.

PiperOrigin-RevId: 602257598
Change-Id: I61e51774611196fb44745dc0aa2dc836b41fcd68
  • Loading branch information
tjgq authored and copybara-github committed Jan 29, 2024
1 parent 19f3154 commit 0297d0f
Show file tree
Hide file tree
Showing 7 changed files with 22 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -268,8 +268,8 @@ public void close() throws IOException {
if (sorted) {
StableSort.stableSort(in, convertedOutputStream);
} else {
while (in.available() > 0) {
SpawnExec ex = SpawnExec.parseDelimitedFrom(in);
SpawnExec ex;
while ((ex = SpawnExec.parseDelimitedFrom(in)) != null) {
convertedOutputStream.write(ex);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@
public final class StableSort {
private static ImmutableList<SpawnExec> read(InputStream in) throws IOException {
ImmutableList.Builder<SpawnExec> result = ImmutableList.builder();
while (in.available() > 0) {
SpawnExec ex = SpawnExec.parseDelimitedFrom(in);
SpawnExec ex;
while ((ex = SpawnExec.parseDelimitedFrom(in)) != null) {
result.add(ex);
}
return result.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -735,8 +735,9 @@ public void testCoverageFileIncluded() throws Exception {

List<BuildEvent> buildEvents = new ArrayList<>();
try (InputStream in = new FileInputStream(buildEventBinaryFile)) {
while (in.available() > 0) {
buildEvents.add(BuildEvent.parseDelimitedFrom(in));
BuildEvent ev;
while ((ev = BuildEvent.parseDelimitedFrom(in)) != null) {
buildEvents.add(ev);
}
}

Expand Down Expand Up @@ -847,8 +848,9 @@ private void testOom(Runnable throwOom) throws Exception {

List<BuildEvent> buildEvents = new ArrayList<>();
try (InputStream in = new FileInputStream(buildEventBinaryFile)) {
while (in.available() > 0) {
buildEvents.add(BuildEvent.parseDelimitedFrom(in));
BuildEvent ev;
while ((ev = BuildEvent.parseDelimitedFrom(in)) != null) {
buildEvents.add(ev);
}
}
Aborted expectedAbort =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -269,8 +269,9 @@ private static ImmutableList<BuildEvent> parseBuildEventsFromBEPStream(File bep)
throws IOException {
ImmutableList.Builder<BuildEvent> buildEvents = ImmutableList.builder();
try (InputStream in = new FileInputStream(bep)) {
while (in.available() > 0) {
buildEvents.add(BuildEvent.parseDelimitedFrom(in));
BuildEvent ev;
while ((ev = BuildEvent.parseDelimitedFrom(in)) != null) {
buildEvents.add(ev);
}
}
return buildEvents.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,8 @@ protected void closeAndAssertLog(SpawnLogContext context, SpawnExec... expected)

ArrayList<SpawnExec> actual = new ArrayList<>();
try (InputStream in = new ZstdInputStream(logPath.getInputStream())) {
while (in.available() > 0) {
ExecLogEntry e = ExecLogEntry.parseDelimitedFrom(in);
ExecLogEntry e;
while ((e = ExecLogEntry.parseDelimitedFrom(in)) != null) {
entryMap.put(e.getId(), e);
if (e.hasSpawn()) {
actual.add(reconstructSpawnExec(e.getSpawn(), entryMap));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ protected void closeAndAssertLog(SpawnLogContext context, SpawnExec... expected)

ArrayList<SpawnExec> actual = new ArrayList<>();
try (InputStream in = logPath.getInputStream()) {
while (in.available() > 0) {
SpawnExec ex = SpawnExec.parseDelimitedFrom(in);
SpawnExec ex;
while ((ex = SpawnExec.parseDelimitedFrom(in)) != null) {
actual.add(ex);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,12 @@ static class FilteringLogParser implements Parser {
public SpawnExec getNext() throws IOException {
SpawnExec ex;
// Find the next record whose runner matches
do {
if (in.available() <= 0) {
// End of file
return null;
while ((ex = SpawnExec.parseDelimitedFrom(in)) != null) {
if (restrictToRunner == null || restrictToRunner.equals(ex.getRunner())) {
return ex;
}
ex = SpawnExec.parseDelimitedFrom(in);
} while (restrictToRunner != null && !restrictToRunner.equals(ex.getRunner()));
return ex;
}
return null;
}
}

Expand Down

0 comments on commit 0297d0f

Please sign in to comment.