RFR: 8364361: [process] java.lang.Process should implement Closeable [v24]

[Pavel Rappo]

On Mon, 27 Oct 2025 18:42:32 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.
>
> Roger Riggs has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Add \@implNote to recommend overriding close and calling `super.close()`.

Looks good.

src/java.base/share/classes/java/lang/Process.java line 121:

> 119:  * or readers, or they have been closed. The Process {@linkplain Process#close close} method closes
> 120:  * all the streams and terminates the process to release the resources. Using try-with-resources to
> 121:  * {@linkplain ProcessBuilder#start()} the process can ensure the process

This link looks weird in the generated documentation:

Using try-with-resources to ProcessBuilder.start() the process can ensure the process is terminated when the try-with-resources block exits.

I suggest fixing the text to just "start" and optionally removing the link as that method is already linked to from this same doc comment:
  

Suggestion:

 * {@linkplain ProcessBuilder#start() start} the process can ensure the process


Related: while it's not part of this PR, consider removing `{@code}` around closed on L130. There's no such method as "closed", and I think readers understand what plain "closed" refers to. Alternatively, you could use this

    {@linkplain #close() closed}

src/java.base/share/classes/java/lang/Process.java line 180:

> 178: 
> 179:     /**
> 180:      * Close all reader and writer streams and wait for the process to terminate.

Suggestion:

     * Closes all reader and writer streams and waits for the process to terminate.

src/java.base/share/classes/java/lang/Process.java line 194:

> 192:      * Closing an already closed stream usually has no effect but is specific to the stream.
> 193:      * If an {@code IOException} occurs when closing a stream it is thrown
> 194:      * after the process has terminated. {@linkplain Exception}s

Suggestion:

     * after the process has terminated. Exceptions

src/java.base/share/classes/java/lang/Process.java line 218:

> 216:      * The {@code errorReader} and {@code errorStream} from the process are closed.
> 217:      * This method {@linkplain #waitFor() waits for the process} to terminate.
> 218:      * If {@linkplain #waitFor() waitFor()} is {@linkplain Thread#interrupt() interrupted}

Suggestion:

     * If {@link #waitFor() waitFor()} is {@linkplain Thread#interrupt() interrupted}


or better still


Suggestion:

     * If wait is {@linkplain Thread#interrupt() interrupted}

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

Marked as reviewed by prappo (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/26649#pullrequestreview-3399134720
PR Review Comment: https://git.openjdk.org/jdk/pull/26649#discussion_r2477745802
PR Review Comment: https://git.openjdk.org/jdk/pull/26649#discussion_r2477762020
PR Review Comment: https://git.openjdk.org/jdk/pull/26649#discussion_r2477816911
PR Review Comment: https://git.openjdk.org/jdk/pull/26649#discussion_r2477789248