Merge pull request #47 from openjavaformat/close-thread-pool

Shut down the command line's thread pool when formatting is done

Author: abashev

open-java-format/src/main/java/com/palantir/javaformat/java/Main.java

@@ -120,11 +120,18 @@ public int format(String... args) throws UsageException {
         }
     }
 
-    @SuppressWarnings("for-rollout:RedundantControlFlow")
     private int formatFiles(CommandLineOptions parameters, JavaFormatterOptions options) {
         int numThreads = Math.min(MAX_THREADS, parameters.files().size());
-        ExecutorService executorService = Executors.newFixedThreadPool(numThreads);
+        // Closing the pool ends its threads, so that a tool that runs Main in-process does not keep them. The close
+        // waits for the submitted tasks, which formatFiles has already waited for.
+        try (ExecutorService executorService = Executors.newFixedThreadPool(numThreads)) {
+            return formatFiles(parameters, options, executorService);
+        }
+    }
 
+    @SuppressWarnings("for-rollout:RedundantControlFlow")
+    private int formatFiles(
+            CommandLineOptions parameters, JavaFormatterOptions options, ExecutorService executorService) {
         Map<Path, String> inputs = new LinkedHashMap<>();
         Map<Path, Future<String>> results = new LinkedHashMap<>();
         boolean allOk = true;

open-java-format/src/test/java/com/palantir/javaformat/java/MainTest.java

@@ -35,6 +35,8 @@
 import java.nio.file.attribute.PosixFilePermission;
 import java.util.EnumSet;
 import java.util.Locale;
+import java.util.concurrent.FutureTask;
+import java.util.concurrent.TimeUnit;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
 import org.junit.jupiter.api.parallel.Execution;
@@ -91,6 +93,29 @@ public void version() throws UsageException {
         assertThat(err.toString()).contains("open-java-format: Version ");
     }
 
+    // Main used to leave its thread pool running after format returned. The command line does not notice, because it
+    // exits, but anything that runs Main in-process kept the idle threads (#40, from google/google-java-format#384).
+    @Test
+    public void formatLeavesNoPoolThreadRunning() throws Exception {
+        Path path = Files.writeString(testFolder.resolve("A.java"), "class A {}\n");
+        Main main = new Main(
+                new PrintWriter(new StringWriter(), true), new PrintWriter(new StringWriter(), true), System.in);
+        // The pool's threads join the thread group of the thread that creates the pool.
+        ThreadGroup group = new ThreadGroup("formatLeavesNoPoolThreadRunning");
+        FutureTask<Integer> format = new FutureTask<>(() -> main.format(path.toString()));
+        new Thread(group, format).start();
+        assertThat(format.get()).isEqualTo(0);
+
+        Thread[] threads = new Thread[group.activeCount() + 16];
+        int count = group.enumerate(threads);
+        for (int i = 0; i < count; i++) {
+            threads[i].join(TimeUnit.SECONDS.toMillis(10));
+            assertWithMessage(threads[i].getName() + " is still running")
+                    .that(threads[i].isAlive())
+                    .isFalse();
+        }
+    }
+
     @Test
     public void preserveOriginalFile() throws Exception {
         Path path = Files.createFile(testFolder.resolve("Test.java"));