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

[Jaikiran Pai]

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

>> Jaikiran Pai has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   use the right include for rlim_t - <sys/resource.h>
>
> src/jdk.jdwp.agent/unix/native/libjdwp/exec_md.c line 129:
> 
>> 127:          * and assume all were opened for the parent process and
>> 128:          * copied over to this child process. we close them all */
>> 129:         const rlim_t max_fd = sysconf(_SC_OPEN_MAX);
> 
> Why not use `int`, like the original code, instead of  `rlim_t` - as per man page `close()` expects `int`, not  `rlim_t`, ex:
> 
> 
>         const int max_fd = (int)sysconf(_SC_OPEN_MAX);
>         JDI_ASSERT(max_fd != -1);
>         int fd;
>          /* leave out standard input/output/error file descriptors */
>          for (fd = STDERR_FILENO + 1; fd < max_fd; fd++) {
>              (void)close(fd);
>          }

Hello Gerard, my understanding is that the limit value configured may exceed the int range. I wanted to avoid the overflow by casting it to int in such cases. I had noticed close() takes an int, but I couldn't think of any other way of avoiding the overflow at this place.

In the JVM parent process we do however limit it to INT_MAX. So maybe I should assume that it will be an int cast it to int here, like you suggest, and add a code comment explaining this? Does that sound OK?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17588#discussion_r1468270331