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

[David Holmes]

On Mon, 29 Jan 2024 19:25:35 GMT, Chris Plummer <cjplummer at openjdk.org> wrote:

>> Jaikiran Pai has updated the pull request incrementally with three additional commits since the last revision:
>> 
>>  - add a log message to help debuggability
>>  - improve code comments
>>  - David's review - use standard isdigit instead of custom isAsciiDigit
>
> src/jdk.jdwp.agent/unix/native/libjdwp/exec_md.c line 129:
> 
>> 127:         /* Find max allowed file descriptors for a process
>> 128:          * and assume all were opened for the parent process and
>> 129:          * copied over to this child process. We close them all. */
> 
> I'm somewhat leaning towards this just being a fatal error. Why would it ever fail? Do we think the version in childproc.c that also does this is doing so for a good reason, or perhaps just like jdwp it used to use the slow version and then kept it around as a backup when the faster version was introduced.

I think we are sliding into " the perfect is the enemy of the good" here. If you want to investigate the resilience of closing FD's then I suggest you look at that in a different PR and let Jai integrate this optimization to avoid the timeouts that were seen.

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

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