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

[Jaikiran Pai]

On Thu, 1 Feb 2024 05:42:07 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Hello David, I had actually first put it in that line you noted, but that then lead to a compilation error:
>> 
>> 
>> In file included from /jdk/src/jdk.jdwp.agent/unix/native/libjdwp/exec_md.c:33:
>> /jdk/src/jdk.jdwp.agent/share/native/libjdwp/error_messages.h:35:20: error: unknown type name 'FILE'
>> void print_message(FILE *fp, const char *prefix,  const char *suffix,
>>                    ^
>> /jdk/src/jdk.jdwp.agent/share/native/libjdwp/error_messages.h:41:29: error: a parameter list without types is only allowed in a function definition
>> const char * jvmtiErrorText(jvmtiError);
>>                             ^
>> /jdk/src/jdk.jdwp.agent/share/native/libjdwp/error_messages.h:43:28: error: a parameter list without types is only allowed in a function definition
>> const char * jdwpErrorText(jdwpError);
>>                            ^
>> 3 errors generated.
>> 
>> 
>> 
>> I think the reason for those compilation errors is that `error_messages.h` and a few other header files are not explicitly listing their necessary includes and are relying on transitive includes. So I decided to take the easy route out, by adding the `error_messages.h` after the `util.h` to rely on the transitive includes to get past that compilation error. 
>> 
>> I think to properly fix that compile error, this following change (which I tested locally and works) would be needed:
>> 
>> 
>> diff --git a/src/jdk.jdwp.agent/share/native/libjdwp/JDWP.h b/src/jdk.jdwp.agent/share/native/libjdwp/JDWP.h
>> index 92a9f457e8a..2683c6791a1 100644
>> --- a/src/jdk.jdwp.agent/share/native/libjdwp/JDWP.h
>> +++ b/src/jdk.jdwp.agent/share/native/libjdwp/JDWP.h
>> @@ -26,6 +26,8 @@
>>  #ifndef JDWP_JDWP_H
>>  #define JDWP_JDWP_H
>>  
>> +#include "jni_md.h"
>> +#include "jvmti.h"
>>  #include "JDWPCommands.h"
>>  
>>  /*
>> diff --git a/src/jdk.jdwp.agent/share/native/libjdwp/error_messages.h b/src/jdk.jdwp.agent/share/native/libjdwp/error_messages.h
>> index 4126b76f226..810ab384f1a 100644
>> --- a/src/jdk.jdwp.agent/share/native/libjdwp/error_messages.h
>> +++ b/src/jdk.jdwp.agent/share/native/libjdwp/error_messages.h
>> @@ -26,6 +26,9 @@
>>  #ifndef JDWP_ERROR_MESSAGES_H
>>  #define JDWP_ERROR_MESSAGES_H
>>  
>> +#include <stdio.h>
>> +#include "JDWP.h"
>> +
>>  /* It is assumed that ALL strings are UTF-8 safe on entry */
>>  #define TTY_MESSAGE(args) ( tty_message args )
>>  #define ERROR_MESSAGE(args) ( \
>> diff --git a/src/jdk.jdwp.agent/unix/native/libjdwp/exec_md.c b/src/jdk.jdwp.agent/unix/native/libjdwp/exec_md.c
>> index b43d4de80...
>
> Please file a JBS issue, but no need for you to address it. Thanks.

I've created https://bugs.openjdk.org/browse/JDK-8325094 to track this.

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

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