Fix race condition on command output streams copy
And add more preconditions to error out if the race condition happens.
This should fix the CI errors on linux.
diff --git a/com.google.devtools.bazel.e4b/src/com/google/devtools/bazel/e4b/command/Command.java b/com.google.devtools.bazel.e4b/src/com/google/devtools/bazel/e4b/command/Command.java
index 594fa39..6217ae0 100644
--- a/com.google.devtools.bazel.e4b/src/com/google/devtools/bazel/e4b/command/Command.java
+++ b/com.google.devtools.bazel.e4b/src/com/google/devtools/bazel/e4b/command/Command.java
@@ -64,11 +64,19 @@
executed = true;
ProcessBuilder builder = new ProcessBuilder(args);
builder.directory(directory);
+ builder.redirectOutput(ProcessBuilder.Redirect.PIPE);
+ builder.redirectError(ProcessBuilder.Redirect.PIPE);
Process process = builder.start();
- copyStream(process.getErrorStream(), stderr);
+ Thread err = copyStream(process.getErrorStream(), stderr);
// seriously? That's stdout, why is it called getInputStream???
- copyStream(process.getInputStream(), stdout);
+ Thread out = copyStream(process.getInputStream(), stdout);
int r = process.waitFor();
+ if (err != null) {
+ err.join();
+ }
+ if (out != null) {
+ out.join();
+ }
synchronized (stderr) {
stderr.close();
}
@@ -78,24 +86,39 @@
return r;
}
- // Launch a thread to copy all data from inputStream to outputStream
- private static void copyStream(InputStream inputStream, OutputStream outputStream) {
- if (outputStream != null) new Thread(new Runnable() {
- @Override
- public void run() {
- byte[] buffer = new byte[4096];
- int read;
- try {
- while ((read = inputStream.read(buffer)) > 0) {
- synchronized (outputStream) {
- outputStream.write(buffer, 0, read);
- }
+ private static class CopyStreamRunnable implements Runnable {
+ private InputStream inputStream;
+ private OutputStream outputStream;
+
+ CopyStreamRunnable(InputStream inputStream, OutputStream outputStream) {
+ this.inputStream = inputStream;
+ this.outputStream = outputStream;
+ }
+
+ @Override
+ public void run() {
+ byte[] buffer = new byte[4096];
+ int read;
+ try {
+ while ((read = inputStream.read(buffer)) > 0) {
+ synchronized (outputStream) {
+ outputStream.write(buffer, 0, read);
}
- } catch (IOException ex) {
- // we simply terminate the thread on exceptions
}
+ } catch (IOException ex) {
+ // we simply terminate the thread on exceptions
}
- }).start();
+ }
+ }
+
+ // Launch a thread to copy all data from inputStream to outputStream
+ private static Thread copyStream(InputStream inputStream, OutputStream outputStream) {
+ if (outputStream != null) {
+ Thread t = new Thread(new CopyStreamRunnable(inputStream, outputStream), "CopyStream");
+ t.start();
+ return t;
+ }
+ return null;
}
/**
diff --git a/com.google.devtools.bazel.e4b/src/com/google/devtools/bazel/e4b/command/SelectOutputStream.java b/com.google.devtools.bazel.e4b/src/com/google/devtools/bazel/e4b/command/SelectOutputStream.java
index 8514d5e..2f7c56e 100644
--- a/com.google.devtools.bazel.e4b/src/com/google/devtools/bazel/e4b/command/SelectOutputStream.java
+++ b/com.google.devtools.bazel.e4b/src/com/google/devtools/bazel/e4b/command/SelectOutputStream.java
@@ -58,6 +58,7 @@
@Override
public void write(int b) throws IOException {
+ Preconditions.checkState(!closed, "Attempted to write on a closed stream");
byte b0 = (byte) b;
if (b0 == '\n') {
select(true);
diff --git a/javatests/com/google/devtools/bazel/e4b/command/CommandTest.java b/javatests/com/google/devtools/bazel/e4b/command/CommandTest.java
index 247933c..426ac34 100644
--- a/javatests/com/google/devtools/bazel/e4b/command/CommandTest.java
+++ b/javatests/com/google/devtools/bazel/e4b/command/CommandTest.java
@@ -85,7 +85,7 @@
assertThat(cmd.run()).isEqualTo(0);
String stdoutStr = new String(stdout.toByteArray(), StandardCharsets.UTF_8).trim();
String stderrStr = new String(stderr.toByteArray(), StandardCharsets.UTF_8).trim();
-
+
assertThat(stdoutStr).isEqualTo("a");
assertThat(stderrStr).isEqualTo("b");
assertThat(cmd.getSelectedErrorLines()).containsExactly("a");
@@ -140,6 +140,7 @@
builder.addArguments("pwd");
Command cmd = builder.build();
assertThat(cmd.run()).isEqualTo(0);
+ assertThat(cmd.getSelectedErrorLines()).isEmpty();
assertThat(cmd.getSelectedOutputLines()).containsExactly(folder.getRoot().getCanonicalPath());
}
}
