Formatter leaks threads and memory #846
Description
The formatter fails to clean up the threads it creates which leaks the threads themselves (if the formatter is run multiple times) and the memory for the threads.
This also can prevent the JVM to from shutting down because the threads are non-daemon threads.
The reason this leak doesn't usually affect users is probably because users run the formatter via the CLI which calls System#exit to kill the JVM process.
Steps to Reproduce:
-
Add this test to
MainTest.java:private static Path createTestSourceFile(TemporaryFolder testFolder, String className) throws IOException { Path path = testFolder.newFile(className + ".java").toPath(); Files.write(path, ("class " + className + " {}\n").getBytes(UTF_8)); try { Files.setPosixFilePermissions(path, EnumSet.of(PosixFilePermission.OWNER_READ)); } catch (UnsupportedOperationException e) { throw new IOException(e); } return path; } @Test public void doesNotLeakThreads() throws Exception { Path testA = createTestSourceFile(testFolder, "TestA"); Path testB = createTestSourceFile(testFolder, "TestB"); // Get the number of threads before formatting. int numberOfThreadsBeforeFormatting = Thread.currentThread().getThreadGroup().activeCount(); // Run the formatter several times. for (int i = 0; i < 10000; i++) { final var errorCode = new GoogleJavaFormatToolProvider() .run( new PrintWriter( new BufferedWriter(new OutputStreamWriter(System.out, UTF_8)), true), new PrintWriter( new BufferedWriter(new OutputStreamWriter(System.err, UTF_8)), true), "--replace", testA.toString(), testB.toString()); assertWithMessage("Error Code").that(errorCode).isEqualTo(0); } // Verify that the number of threads is nearly the same before and after. NB: we allow for a few // additional threads since we don't completely control the number of running threads in this // test. assertThat(Thread.currentThread().getThreadGroup().activeCount()) .isLessThan(numberOfThreadsBeforeFormatting + 5); } -
Run the test:
mvn test -Dtest="MainTest#doesNotLeakThreads"
If the test fails you'll get an OOM error:
[WARNING] Corrupted STDOUT by directly writing to native stream in forked JVM 1. See FAQ web page and the dump file /Volumes/Projects/google/google-java-format/core/target/surefire-reports/2022-10-15T16-48-29_971-jvmRun1.dumpstream
[ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 1.654 s <<< FAILURE! - in com.google.googlejavaformat.java.MainTest
[ERROR] doesNotLeakThreads(com.google.googlejavaformat.java.MainTest) Time elapsed: 1.629 s <<< ERROR!
java.lang.OutOfMemoryError: unable to create native thread: possibly out of memory or process/resource limits reached
at com.google.googlejavaformat.java.MainTest.doesNotLeakThreads(MainTest.java:645)
java.lang.OutOfMemoryError: unable to create native thread: possibly out of memory or process/resource limits reached
at java.base/java.lang.Thread.start0(Native Method)
at java.base/java.lang.Thread.start(Thread.java:798)
at java.base/java.util.concurrent.ThreadPoolExecutor.addWorker(ThreadPoolExecutor.java:937)
at java.base/java.util.concurrent.ThreadPoolExecutor.ensurePrestart(ThreadPoolExecutor.java:1583)
at java.base/java.util.concurrent.ScheduledThreadPoolExecutor.delayedExecute(ScheduledThreadPoolExecutor.java:346)
at java.base/java.util.concurrent.ScheduledThreadPoolExecutor.schedule(ScheduledThreadPoolExecutor.java:562)
at org.apache.maven.surefire.booter.ForkedBooter.launchLastDitchDaemonShutdownThread(ForkedBooter.java:369)
at org.apache.maven.surefire.booter.ForkedBooter.acknowledgedExit(ForkedBooter.java:333)
at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:145)
at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:418)
Exception in thread "main" java.lang.OutOfMemoryError: unable to create native thread: possibly out of memory or process/resource limits reached
at java.base/java.lang.Thread.start0(Native Method)
at java.base/java.lang.Thread.start(Thread.java:798)
at java.base/java.util.concurrent.ThreadPoolExecutor.addWorker(ThreadPoolExecutor.java:937)
at java.base/java.util.concurrent.ThreadPoolExecutor.ensurePrestart(ThreadPoolExecutor.java:1583)
at java.base/java.util.concurrent.ScheduledThreadPoolExecutor.delayedExecute(ScheduledThreadPoolExecutor.java:346)
at java.base/java.util.concurrent.ScheduledThreadPoolExecutor.schedule(ScheduledThreadPoolExecutor.java:562)
at org.apache.maven.surefire.booter.ForkedBooter.launchLastDitchDaemonShutdownThread(ForkedBooter.java:369)
at org.apache.maven.surefire.booter.ForkedBooter.exit(ForkedBooter.java:316)
at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:425)
If the test passes the bug is fixed.