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 654:
> 652: // Close each stream
> 653: quietClose(outputWriter != null ? outputWriter :
> getOutputStream());
> 654: quietClose(inputReader != null ? inputReader :
> getInputStream());
Suggestion:
quietClose(inputReader != null ? inputReader : getInputStream());
test/jdk/java/lang/Process/ProcessCloseTest.java line 61:
> 59: JAVA_HOME = System.getProperty("JAVA_HOME");
> 60: String classPath = System.getProperty("test.class.path");
> 61: return List.of(JAVA_HOME + "/bin/java", "-cp", classPath,
> ProcessCloseTest.class.getName());
Suggestion:
return List.of(JAVA_HOME + "/bin/java", "-cp", classPath,
ProcessCloseTest.class.getName());
test/jdk/java/lang/Process/ProcessCloseTest.java line 172:
> 170: c.command.accept(p);
> 171: });
> 172: } catch (IOException ioe) {
Suggestion:
} catch (IOException ioe) {
test/jdk/java/lang/Process/ProcessCloseTest.java line 307:
> 305: private static void stderrExpectPolo(Process p) {
> 306: String line = readLine(p.getErrorStream());
> 307: Assertions.assertEquals("Polo", line, "Stderr Expected
> Empty"); }
Suggestion:
Assertions.assertEquals("Polo", line, "Stderr Expected Empty");
}
test/jdk/java/lang/Process/ProcessCloseTest.java line 316:
> 314: private static void stderrExpectEmpty(Process p) {
> 315: String line = readLine(p.getErrorStream());
> 316: Assertions.assertEquals("", line, "Stderr Expected Polo");
> }
Suggestion:
Assertions.assertEquals("", line, "Stderr Expected Polo");
}
test/jdk/java/lang/Process/ProcessCloseTest.java line 470:
> 468: * @param childCommands a sequence of ChildCommand names.
> 469: */
> 470: public static void main(String... childCommands) {
Suggestion:
public static void main(String... childCommands) {
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26649#discussion_r2269061968
PR Review Comment: https://git.openjdk.org/jdk/pull/26649#discussion_r2269057839
PR Review Comment: https://git.openjdk.org/jdk/pull/26649#discussion_r2269058295
PR Review Comment: https://git.openjdk.org/jdk/pull/26649#discussion_r2269059672
PR Review Comment: https://git.openjdk.org/jdk/pull/26649#discussion_r2269060271
PR Review Comment: https://git.openjdk.org/jdk/pull/26649#discussion_r2269060904
