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

[Gerard Ziemski]

On Sat, 27 Jan 2024 01:18:09 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> 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?

That is fine, but in that case we should also do:

`JDI_ASSERT(max_fd <= INT_MAX);`

in case `sysconf(_SC_OPEN_MAX)` returns value greater than `INT_MAX`, since `close()` only accepts `int`.

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

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