On Tue, 5 Aug 2025 10:32:42 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 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. ------------- PR Review: https://git.openjdk.org/jdk/pull/26568#pullrequestreview-3092000666
