On Thu, 7 Aug 2025 22:05:55 GMT, Roger Riggs <rri...@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:
> 
>   Updates from review comments:
>    - Editorial improvements to javadoc
>    - Exceptions that occur closing streams are quietly logged
>      to the java.lang.Process system log as DEBUG
>    - The prototype code attempting to wait for process exit is removed,
>      it provided marginal benefit and raised complexity due to interrupt 
> handling
>    - Test updates for racy test cases that are not errors

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

> 653:         quietClose(outputWriter != null ? outputWriter : 
> getOutputStream());
> 654:         quietClose(inputReader != null ? inputReader  : 
> getInputStream());
> 655:         quietClose(errorReader != null ? errorReader : getErrorStream());

Async close of process stream is a complicated here. Do you have a summary on 
how it behaves on both Unix and Windows? I'm thinking about the deferred close 
code in particular. (I don't have an issue with close throwing away the 
exceptions, I'm just not 100% sure that async close is reliable in the first 
place).

> Closing an already closed stream usually has no effect but is specific to the 
> stream.

I wonder if we could specify Process::close to be idempotent. That would mean 
we couldn't attempt to re-close if a first close of stream failed or had some 
other side effect.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26649#discussion_r2262608881

Reply via email to