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

[Guanqiang Han]

On Wed, 6 Aug 2025 10:55:07 GMT, Kevin Walls <kevinw at openjdk.org> wrote:

>> Guanqiang Han has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision:
>> 
>>  - a small fix
>>  - Merge remote-tracking branch 'upstream/master' into 8364312
>>  - Update exec_md.c
>>    
>>    correct an compilation error
>>  - 8364312: debug agent should set FD_CLOEXEC flag rather than explicitly closing every open file
>>    
>>    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().
>
> Hi -
> 
> (The code duplication from src/java.base/unix/native/libjava/childproc.c is unfortunate but maybe it's hard to avoid.)
> 
> exec_md.c has the existing closeDescriptors() that "returns 0 on failure and 1 on success." which is unusual and unfortunate as zero is a typical success value.
> 
> As this is only used locally, this should be an opportunity to change that, and have the new markDescriptorsCloseOnExec() return more normal values, like 0 for success and -1 for failure (like in childproc.c which this borrows from).
> (Unless Chris disagrees or sees wider usage...)
> 
> 
> Another existing problem:
> forkedChildProcess() comments that it will: "Close all file descriptors that have been copied over"
> But it calls closeDescriptors() which says it: "Closes every file descriptor that is listed as a directory"
> 
> Can we make the comments consistent while we are here? 8-)
> 
> The comments before forkedChildProcess() at line 118 and within it at 124 talk about actual closing, not marking as close on exec.

Hi @kevinjwalls, thank you for the detailed feedback — much appreciated!

I agree with your suggestions and have updated the PR accordingly.

Please take another look when you have a moment.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/26568#issuecomment-3159927768