On Mon, 22 Sep 2025 15:40:06 GMT, Joachim Kern <jk...@openjdk.org> wrote:
>> The new tests >> java/lang/ProcessBuilder/FDLeakTest/FDLeakTest.java#fork >> java/lang/ProcessBuilder/FDLeakTest/FDLeakTest.java#posix_spawn >> fail on AIX. >> The tests were added with >> [JDK-8210549](https://bugs.openjdk.org/browse/JDK-8210549) . >> >> Error output is >> java/lang/ProcessBuilder/FDLeakTest/FDLeakTest.java#fork >> >> Opened and leaked ./testfile_FDLeaker.txt (4) >> ----------System.err:(13/647)---------- >> *** Parent leaked file descriptor 9 *** >> *** Parent leaked file descriptor 10 *** >> java.lang.RuntimeException: Failed >> at FDLeakTest.main(FDLeakTest.java:69) >> at >> java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104) >> at java.base/java.lang.reflect.Method.invoke(Method.java:565) >> at >> com.sun.javatest.regtest.agent.MainWrapper$MainTask.run(MainWrapper.java:138) >> at java.base/java.lang.Thread.run(Thread.java:1474) >> >> >> java/lang/ProcessBuilder/FDLeakTest/FDLeakTest.java#posix_spawn >> >> >> ----------System.out:(1/46)---------- >> Opened and leaked ./testfile_FDLeaker.txt (4) >> ----------System.err:(13/647)---------- >> *** Parent leaked file descriptor 9 *** >> *** Parent leaked file descriptor 10 *** >> java.lang.RuntimeException: Failed >> at FDLeakTest.main(FDLeakTest.java:69) >> at >> java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104) >> at java.base/java.lang.reflect.Method.invoke(Method.java:565) >> at >> com.sun.javatest.regtest.agent.MainWrapper$MainTask.run(MainWrapper.java:138) >> at java.base/java.lang.Thread.run(Thread.java:1474) > > Joachim Kern has updated the pull request incrementally with one additional > commit since the last revision: > > cosmetic changes @JoKern65 I modified your patch somewhat: - the comment now highlights the difference between AIX and other OSes, and the technique AIX uses - the change now uses FAIL_FILENO instead of relying on the wonky stderr+1 logic src/java.base/unix/native/libjava/childproc.c line 99: > 97: * It should not survive a passing exec(). So we can set the > close_on_exec > 98: * flag for FD 3. FDs 0,1 and 2 should survive the exec(), we do not > change them. > 99: */ Suggestion: /* On AIX, we cannot rely on proc file system iteration to find all open files. Since * iteration over all possible file descriptors, and subsequently closing them, can * take a very long time, we use a bulk close via `ioctl` that is available on AIX. * Since we hard-close, we need to make sure to keep the fail pipe file descriptor * alive until the exec call. Therefore we mark the fail pipe fd with close on exec * like the other OSes do, but then proceed to hard-close file descriptors beyond that. */ src/java.base/unix/native/libjava/childproc.c line 101: > 99: */ > 100: if (fcntl(STDERR_FILENO + 2, F_CLOSEM, 0) == -1 || > 101: (markCloseOnExec(STDERR_FILENO + 1) == -1 && errno != EBADF)) { Suggestion: if (fcntl(FAIL_FILENO + 1, F_CLOSEM, 0) == -1 || (markCloseOnExec(FAIL_FILENO) == -1 && errno != EBADF)) { ------------- PR Review: https://git.openjdk.org/jdk/pull/27291#pullrequestreview-3258098659 PR Review Comment: https://git.openjdk.org/jdk/pull/27291#discussion_r2372399567 PR Review Comment: https://git.openjdk.org/jdk/pull/27291#discussion_r2372406427
