RFR: 8324668: JDWP process management needs more efficient file descriptor handling [v2]

[Jaikiran Pai]

On Fri, 26 Jan 2024 17:33:39 GMT, Gerard Ziemski <gziemski at openjdk.org> wrote:

> Have you considered using `fcntl(d, F_SETFD, 1)` instead of the fancy `closeDescriptors()`?
> 
> I haven't tested it myself, but per the `man close` page:
> 
> ```
> Most of the descriptors can be rearranged with dup2(2) or deleted with close() before the execve is attempted, but if some of these descriptors will still be needed if the execve fails, it is necessary to arrange for them to be closed if the execve succeeds.  For this reason, the call “fcntl(d, F_SETFD, 1)” is provided, which arranges that a descriptor will be closed after a successful execve;
> ```

I hadn't considered that. Now that you mentioned it, I had a look at what `man close` says and `man fcntl` says. My understanding of it is that we use `fcntl(fd, F_SETFD, FD_CLOEXEC)` (the `1` in the example corresponds to `FD_CLOEXEC`) if we want to close a specific file descriptor only if a subsequent call to `execvp` (or similar exec function) succeeds. If the subsequent call to `execvp` fails, then the file descriptor won't be closed. From what I understand of the code in jdwp, I don't think we need that behaviour here. Plus using `fcntl` won't get rid of our `closeDescriptors()` function. We will still need it, it's just the call to `close()` within the loop that gets replaced by a call to `fcntl`.

> Should this be:
> (void)close(fd);
> and
> (void)closedir(dp);
> to show that we're ignoring the return value?

Done. I've updated the PR to cast these calls to void.

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

PR Comment: https://git.openjdk.org/jdk/pull/17588#issuecomment-1913151651
PR Review Comment: https://git.openjdk.org/jdk/pull/17588#discussion_r1468462470