diff --git a/open-java-format/src/main/java/com/palantir/javaformat/java/Main.java b/open-java-format/src/main/java/com/palantir/javaformat/java/Main.java index a05188668..a3b53566f 100644 --- a/open-java-format/src/main/java/com/palantir/javaformat/java/Main.java +++ b/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 inputs = new LinkedHashMap<>(); Map> results = new LinkedHashMap<>(); boolean allOk = true; diff --git a/open-java-format/src/test/java/com/palantir/javaformat/java/MainTest.java b/open-java-format/src/test/java/com/palantir/javaformat/java/MainTest.java index d9f12853b..a87238af0 100644 --- a/open-java-format/src/test/java/com/palantir/javaformat/java/MainTest.java +++ b/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 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"));