From e56989f132038b1e1508edfe0875a15f0303b7b8 Mon Sep 17 00:00:00 2001 From: Googler Date: Sun, 28 Jan 2024 20:21:37 -0800 Subject: [PATCH] [7.1.0] Avoid using `InputStream.available()` to detect EOF while reading delimited 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 --- .../build/lib/exec/ExpandedSpawnLogContext.java | 4 ++-- .../google/devtools/build/lib/exec/StableSort.java | 4 ++-- .../BazelBuildEventServiceModuleTest.java | 10 ++++++---- .../build/lib/buildtool/TargetCompleteEventTest.java | 5 +++-- .../build/lib/exec/CompactSpawnLogContextTest.java | 4 ++-- .../build/lib/exec/ExpandedSpawnLogContextTest.java | 4 ++-- .../google/devtools/build/execlog/ExecLogParser.java | 12 +++++------- 7 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/exec/ExpandedSpawnLogContext.java b/src/main/java/com/google/devtools/build/lib/exec/ExpandedSpawnLogContext.java index 4cb72217e5d61e..9725c08732d17a 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/ExpandedSpawnLogContext.java +++ b/src/main/java/com/google/devtools/build/lib/exec/ExpandedSpawnLogContext.java @@ -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); } } diff --git a/src/main/java/com/google/devtools/build/lib/exec/StableSort.java b/src/main/java/com/google/devtools/build/lib/exec/StableSort.java index fb3bfa97d12f18..c5a2c0ab905c0a 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/StableSort.java +++ b/src/main/java/com/google/devtools/build/lib/exec/StableSort.java @@ -37,8 +37,8 @@ public final class StableSort { private static ImmutableList read(InputStream in) throws IOException { ImmutableList.Builder 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(); diff --git a/src/test/java/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModuleTest.java b/src/test/java/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModuleTest.java index 1115c3612d1c78..6a177424add7dd 100644 --- a/src/test/java/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModuleTest.java +++ b/src/test/java/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModuleTest.java @@ -730,8 +730,9 @@ public void testCoverageFileIncluded() throws Exception { List 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); } } @@ -842,8 +843,9 @@ private void testOom(Runnable throwOom) throws Exception { List 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 = diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/TargetCompleteEventTest.java b/src/test/java/com/google/devtools/build/lib/buildtool/TargetCompleteEventTest.java index 2ac7acad8ce75f..86882f2ff6bb04 100644 --- a/src/test/java/com/google/devtools/build/lib/buildtool/TargetCompleteEventTest.java +++ b/src/test/java/com/google/devtools/build/lib/buildtool/TargetCompleteEventTest.java @@ -269,8 +269,9 @@ private static ImmutableList parseBuildEventsFromBEPStream(File bep) throws IOException { ImmutableList.Builder 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(); diff --git a/src/test/java/com/google/devtools/build/lib/exec/CompactSpawnLogContextTest.java b/src/test/java/com/google/devtools/build/lib/exec/CompactSpawnLogContextTest.java index 6ebb4985043a23..52d21f3cf0e952 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/CompactSpawnLogContextTest.java +++ b/src/test/java/com/google/devtools/build/lib/exec/CompactSpawnLogContextTest.java @@ -131,8 +131,8 @@ protected void closeAndAssertLog(SpawnLogContext context, SpawnExec... expected) ArrayList 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)); diff --git a/src/test/java/com/google/devtools/build/lib/exec/ExpandedSpawnLogContextTest.java b/src/test/java/com/google/devtools/build/lib/exec/ExpandedSpawnLogContextTest.java index c3f74b7f4acab3..732d88992c93b8 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/ExpandedSpawnLogContextTest.java +++ b/src/test/java/com/google/devtools/build/lib/exec/ExpandedSpawnLogContextTest.java @@ -59,8 +59,8 @@ protected void closeAndAssertLog(SpawnLogContext context, SpawnExec... expected) ArrayList 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); } } diff --git a/src/tools/execlog/src/main/java/com/google/devtools/build/execlog/ExecLogParser.java b/src/tools/execlog/src/main/java/com/google/devtools/build/execlog/ExecLogParser.java index d6a6291652986f..cbc37283e57e03 100644 --- a/src/tools/execlog/src/main/java/com/google/devtools/build/execlog/ExecLogParser.java +++ b/src/tools/execlog/src/main/java/com/google/devtools/build/execlog/ExecLogParser.java @@ -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; } }