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

[Jaikiran Pai]

On Sat, 27 Jan 2024 13:06:59 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> src/jdk.jdwp.agent/unix/native/libjdwp/exec_md.c line 112:
>> 
>>> 110:     }
>>> 111: 
>>> 112:     closedir(dp);
>> 
>> Should this be:
>> 
>> `(void)close(fd);`
>> 
>> and 
>> 
>> `(void)closedir(dp);`
>> 
>> to show that we're ignoring the return value?
>> 
>> Since this area is pretty mysterious to deal with, should we consider checking whether `closedir()` actually returns am error, and at least be verbal and print a warning, as opposed to silently ignoring it?
>> 
>> Printing a warning, might help us diagnose any issue in the future?
>
>> 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.

>Since this area is pretty mysterious to deal with, should we consider checking whether closedir() actually returns am error, and at least be verbal and print a warning, as opposed to silently ignoring it?
Printing a warning, might help us diagnose any issue in the future?

I think printing a warning if closedir() fails may not be useful and may be misleading too. But your point about logging a warning in this area sounds reasonable. I think what we could log as a warning is if we fallback to the slow sequential close() approach if/when this optimal closeDescriptor() implementation fails. I've updated the PR with such a log message. Do you think that would be enough?

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

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