RFR: 8364361: [process] java.lang.Process should implement close and be AutoCloseable [v2]

Andrey Turbanov aturbanov at openjdk.org
Tue Aug 12 08:16:21 UTC 2025


On Thu, 7 Aug 2025 22:05:55 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:
> 
>   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


More information about the core-libs-dev mailing list