RFR: 8364312: debug agent should set FD_CLOEXEC flag rather than explicitly closing every open file [v2]

[Chris Plummer]

On Thu, 31 Jul 2025 09:51:35 GMT, Guanqiang Han <ghan at 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