On Mon, 29 Sep 2025 14:36:46 GMT, Roger Riggs <[email protected]> wrote:
>> src/java.base/share/classes/java/lang/Process.java line 186:
>>
>>> 184: * <p>
>>> 185: * Before calling this method the caller should read the streams
>>> for any
>>> 186: * data or text and call {@linkplain #waitFor() waitFor} if the
>>> exit value is needed.
>>
>> Should we add some additional text which clearly highlights that waitFor()
>> needs to be called (before close()) to allow the process to run to
>> completion, even when the exit value or the stream contents aren't used/read
>> by the calling application? I realize that a few lines above we specify:
>>
>>> The streams are closed immediately and the process is terminated without
>>> waiting.
>>
>> But maybe explicitly saying that waitFor() is necessary for allowing the
>> process to run to completion would help reduce confusion? When I first saw
>> the PR, my initial thought was `Process.close()` would wait for the process
>> to run to completion and cleanup the resources that were used and I suspect
>> it might become a common pattern where applications do something like:
>>
>>
>> try (Process p = new ProcessBuilder("some process that takes 5 seconds to
>> complete normally").start()) {
>> }
>>
>> and be surprised that the launched process was abruptly terminated. So maybe
>> some clear text on this method would help?
>
> I think those warnings are already present.
> - `process is terminated without waiting`
> - `the caller should read the streams for any data or text and call waitFor
> if the exit value is needed`
> - `Calling waitFor before calling close or exiting the try-with-resources
> block allows the process time to clean up and exit.`
> - and the example calls waitFor inside the try-with-resources.
>
> Does this add the emphasis you are asking for:
> `Calling waitFor after close() or after the try-with-resources block exits
> returns the status after destroying the process.`
I read the current proposed text a few more times. What you state is true -
that it already covers the part that this method won't wait for the process to
run to completion. However, I think, that detail doesn't register strongly when
reading this text.
Here's what I had in mind - instead of the following sentence:
* Before calling this method the caller should read the streams for any
* data or text and call {@linkplain #waitFor() waitFor} if the exit value is
needed.
would something like this be appropriate:
* If the process should be allowed to run to completion, or the data from the
* process stream(s) or the exit value of the process is of interest, then
* the caller must {@linkplain #waitFor() wait} for the process to complete
* before calling this method.
(the rest of the text in the method doc can stay as-is)
I think this expressly states that the applications must not call this
`close()` method if they wish the process to run to completion.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26649#discussion_r2391939947
