On Thu, 31 Jul 2025 09:51:35 GMT, Guanqiang Han <g...@openjdk.org> wrote:
>> Create a new function that marks all file descriptors found in /proc/self/fd
>> with the FD_CLOEXEC flag to ensure they are automatically closed upon
>> execution of a new program via exec().
>
> Guanqiang Han has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Update exec_md.c
>
> correct an compilation error
I added some comments. Testing needs to be done on all of the following
test/hotspot/jtreg/vmTestbase/nsk/jdi
test/hotspot/jtreg/vmTestbase/nsk/jdb
test/hotspot/jtreg/vmTestbase/nsk/jdwp
test/jdk/com/sun/jdi
Are you able to test all supported platforms (linux-x64, linux-aarch64,
macosx-x64, macosx-aarch64, windows-x64)? If you can't, I'll download your
patch and run the changes through our CI.
src/jdk.jdwp.agent/unix/native/libjdwp/exec_md.c line 127:
> 125: /* Close all file descriptors that have been copied over
> 126: * from the parent process due to fork(). */
> 127: if (markDescriptorsCloseOnExec() != 1) { /* failed, close the old
> way */
Lines 135 to 138 need to be updated below. Since we are no longer calling
closeDescriptors(), STDERR_FILENO +1 and +2 are no longer already closed. The
comment needs updating, and you need to set `i` to `STDERR_FILENO + 1`. Also,
you should test this code by forcing markDescriptorsCloseOnExec() to always
fail.
I'd also suggest doing a test run with a JDI_ASSERT(JNI_FALSE) if
markDescriptorsCloseOnExec() fails. If FD_CLOEXEC is supported, it should
always be succeeding.
Also, the "old way" in childproc.c still uses FD_CLOEXEC. The only difference
is rather than iterating over the directory contents it instead just iterates
over every possible fd. So it never makes use of close() as is being done below.
-------------
Changes requested by cjplummer (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/26568#pullrequestreview-3084199267
PR Review Comment: https://git.openjdk.org/jdk/pull/26568#discussion_r2251485425
