RFR: 8364361: [process] java.lang.Process should implement close and be AutoCloseable

[Volkan Yazici]

On Tue, 5 Aug 2025 18:21:24 GMT, Roger Riggs <rriggs at openjdk.org> wrote:

> The teardown of a Process launched by `ProcessBuilder` includes the closing of streams and ensuring the termination of the process is the responsibility of the caller. The `Process.close()` method provides a clear and obvious way to ensure all the streams are closed and the process terminated.
> 
> The try-with-resources statement is frequently used to open streams and ensure they are closed on exiting the block. By implementing `AutoClosable.close()` the completeness of closing the streams and process termination can be done by try-with-resources.
> 
> The actions of the `close()` method are to close each stream and destroy the process if it has not terminated.

src/java.base/share/classes/java/lang/snippet-files/ProcessExamples.java line 38:

> 36:                 Paddling with the river flow;
> 37:                 Chilling still, go slow.
> 38:                 """;

Nit: I'm not against Haiku, though `writer.write("Hello, world!");` can be enough to get the message across and save the reader (and the maintainer) 6 LoC.

src/java.base/share/classes/java/lang/snippet-files/ProcessExamples.java line 48:

> 46:             // Read all lines and print each
> 47:             List<String> lines = reader.readAllLines();
> 48:             lines.forEach(System.err::println);

Simplification suggestion:

Suggestion:

            reader.readAllLines().forEach(System.err::println);


Note that this will render the `j.u.List` import at the top redundant too.

src/java.base/share/classes/java/lang/snippet-files/ProcessExamples.java line 51:

> 49:             var status = p.waitFor();
> 50:             if (status != 0)
> 51:                 throw new RuntimeException("process status: " + status);

Suggestion:

                throw new RuntimeException("unexpected process status: " + status);

src/java.base/share/classes/java/lang/snippet-files/ProcessExamples.java line 53:

> 51:                 throw new RuntimeException("process status: " + status);
> 52:         } catch (Throwable t) {
> 53:             System.out.println("Process failed: " + t);

Shall we instead catch `Exception` here? Quoting from Effective Java:

> Do not catch `Throwable`. It is wrong to catch `Throwable`: it includes errors (such as `OutOfMemoryError`, `StackOverflowError`, etc.) that are not meant to be caught by applications.

Suggestion:

        } catch (Exception e) {
            System.out.println("Process failed: " + e);

test/jdk/java/lang/Process/ProcessCloseTest.java line 47:

> 45:  * @bug 8336479
> 46:  * @summary Tests for Process.close
> 47:  * @library /test/lib

Unless I'm mistaken, there are no `/test/lib` dependencies:

Suggestion:

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/26649#discussion_r2256334744
PR Review Comment: https://git.openjdk.org/jdk/pull/26649#discussion_r2256356929
PR Review Comment: https://git.openjdk.org/jdk/pull/26649#discussion_r2256342566
PR Review Comment: https://git.openjdk.org/jdk/pull/26649#discussion_r2256350644
PR Review Comment: https://git.openjdk.org/jdk/pull/26649#discussion_r2256365508