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

Jason Mehrens duke at openjdk.org
Wed Oct 8 14:02:47 UTC 2025


On Wed, 8 Oct 2025 05:26:37 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> Roger Riggs has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 22 additional commits since the last revision:
>> 
>>  - The ProcessCloseTest is modified to be able to test the logic
>>    around close() waiting for the process to exit and the specified
>>    behavior of interrupting the waitFor.
>>  - Close is modified to wait for the process to terminate before returning.
>>    Please review the revised javadoc.
>>    As suggested in review comments, waiting for the process to terminate
>>    allows a more orderly cleanup by the application.
>>    The streams are closed and close uses `waitFor()` with an unlimited timeout
>>    for the process to terminate.
>>    While waiting the thread can be `interrupted` to exit the `waitFor`.
>>    If/when `waitFor` is interrupted, the process is destroyedForcibly on
>>    all platforms. When close() returns, the thread interrupt will pending
>>    and can be handled by the caller.
>>    If a watchdog timeout is desired on close(), a separate thread
>>    can schedule an interrupt at a suitable time after close is called.
>>  - Review comment improvements to make expected behavior clearer.
>>  - Merge branch 'master' into 8364361-process-autocloseable
>>  - Clarify the use of waitFor after close() or T-W-R exit.
>>  - Additional review feedback.
>>    Updated to emphasize reading the data from the streams and calling waitFor
>>    to allow for normal termination.
>>  - Merge branch 'master' into 8364361-process-autocloseable
>>  - Address review comments for code and javadoc in Process, the test and the example.
>>    Reinforced that calling waitFor should occur before calling close to wait for termination and get the exitStatus.
>>    Corrected the error message check for invalid handles on Windows.
>>    Update test to allow normal completion when exceptions may be expected.
>>    Correct the expected message on Windows for an invalid handle exception.
>>  - Update close() to use "terminate" consistently.
>>  - Remove volatile from "closed" field; updates are guarded by synchronized.
>>  - ... and 12 more: https://git.openjdk.org/jdk/compare/91574b3b...6f8f7327
>
> src/java.base/share/classes/java/lang/Process.java line 235:
> 
>> 233:                 destroyForcibly();
>> 234:                 // Re-assert the interrupt
>> 235:                 Thread.currentThread().interrupt();
> 
> I think this is still problematic as code after the t-w-r can't be guaranteed that the child has terminated. So I think it needs a loop that ends when waitFor completes without an exception (destroyForcibly would only be called on the first interrupt of course).

I also wonder if close of streams should happen after first wait on destroy or on interruption. I'm thinking the difference is that streams would still be usable to fetch buffered data.

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

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


More information about the core-libs-dev mailing list