Ensure stream gobblers are properly finished

Author: henryju

lib/src/main/java/org/sonarsource/scanner/lib/internal/JavaRunner.java

@@ -22,11 +22,9 @@
 import com.google.gson.Gson;
 import com.google.gson.annotations.SerializedName;
 import java.io.BufferedReader;
-import java.io.BufferedWriter;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
-import java.io.OutputStream;
 import java.io.OutputStreamWriter;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Path;
@@ -56,17 +54,22 @@ public void execute(List<String> args, @Nullable String input) {
     try {
       List<String> command = new ArrayList<>(args);
       command.add(0, javaExecutable.toString());
+      LOG.atDebug().addArgument(() -> String.join(" ", command)).log("Executing: {}");
       Process process = new ProcessBuilder(command).start();
       if (input != null) {
-        try (OutputStream stdin = process.getOutputStream();
-          BufferedWriter writer = new BufferedWriter(new OutputStreamWriter(stdin, StandardCharsets.UTF_8))) {
-          writer.write(input);
+        try (var stdin = process.getOutputStream(); var osw = new OutputStreamWriter(stdin, StandardCharsets.UTF_8)) {
+          osw.write(input);
         }
       }
-      new StreamGobbler(process.getInputStream(), this::tryParse).start();
-      new StreamGobbler(process.getErrorStream(), stderr -> LOG.error("[stderr] {}", stderr)).start();
-      if (process.waitFor() != 0) {
-        throw new IllegalStateException("Error returned by the Java command execution");
+      var stdoutConsummer = new StreamGobbler(process.getInputStream(), this::tryParse);
+      var stdErrConsummer = new StreamGobbler(process.getErrorStream(), stderr -> LOG.error("[stderr] {}", stderr));
+      stdErrConsummer.start();
+      stdoutConsummer.start();
+      var exitCode = process.waitFor();
+      stdoutConsummer.join();
+      stdErrConsummer.join();
+      if (exitCode != 0) {
+        throw new IllegalStateException("Error returned by the Java command execution: " + process.exitValue());
       }
     } catch (IOException | InterruptedException e) {
       Thread.currentThread().interrupt();

lib/src/test/java/org/sonarsource/scanner/lib/internal/JavaRunnerTest.java

@@ -28,7 +28,6 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
-import static org.awaitility.Awaitility.await;
 
 class JavaRunnerTest {
 
@@ -40,14 +39,10 @@ void execute_shouldLogProcessOutput() {
     JavaRunner runner = new JavaRunner(Paths.get("java"), JreCacheHit.DISABLED);
 
     runner.execute(List.of("--version"), "test");
-    await().untilAsserted(() -> {
-      assertThat(logTester.logs(Level.INFO)).isNotEmpty().allMatch(s -> s.startsWith("[stdout] "));
-    });
+    assertThat(logTester.logs(Level.INFO)).isNotEmpty().allMatch(s -> s.startsWith("[stdout] "));
 
     runner.execute(List.of("-version"), null);
-    await().untilAsserted(() -> {
-      assertThat(logTester.logs(Level.ERROR)).isNotEmpty().allMatch(s -> s.startsWith("[stderr] "));
-    });
+    assertThat(logTester.logs(Level.ERROR)).isNotEmpty().allMatch(s -> s.startsWith("[stderr] "));
   }
 
   @Test