On Mon, 29 Jan 2024 19:25:35 GMT, Chris Plummer <cjplum...@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

Reply via email to