RFR: 8291917: Windows - Improve error messages when the C Runtime Libraries or jvm.dll cannot be loaded [v4]

Julian Waters jwaters at openjdk.org
Fri Oct 7 13:39:54 UTC 2022


On Sun, 7 Aug 2022 07:57:49 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Nothing I could find in the tests that suggest they use this message as input, and none of them have failed with this patch, so I guess that's a good thing? :P
>> 
>> I am slightly concerned with going the route of 2 different calls for WIN32 and the C runtime, since `JLI_ReportErrorMessageSys` is a shared function, only the implementations differ from platform to platform. I can't think of any solution off the top of my head to implement such a change without drastically changing either the Unix variant as well, or the declaration's signature in the shared header unfortunately.
>> 
>> I was initially hesitant to change the formatting of spacing between the caller's message and system error, since the original intention for leaving it up to the caller may have been to allow for better flexibility. Also a concern was any behaviour differences that might result with the Unix variant, but it seems like the 2 format their messages entirely differently - While Windows appends the system error to the end of message without any formatting, Unix prints it on an entirely separate line above the message the caller passed it:
>> 
>> JNIEXPORT void JNICALL
>> JLI_ReportErrorMessageSys(const char* fmt, ...) {
>>     va_list vl;
>>     char *emsg;
>> 
>>     /*
>>      * TODO: its safer to use strerror_r but is not available on
>>      * Solaris 8. Until then....
>>      */
>>     emsg = strerror(errno);
>>     if (emsg != NULL) {
>>         fprintf(stderr, "%s\n", emsg);
>>     }
>> 
>>     va_start(vl, fmt);
>>     vfprintf(stderr, fmt, vl);
>>     fprintf(stderr, "\n");
>>     va_end(vl);
>> }
>> 
>> 
>>> If you can make the fix for the CRT extra info small, I'd go for it.
>> 
>> I don't quite get what you mean by that, should I revert the changes made to the freeit checks?
>
>> Nothing I could find in the tests that suggest they use this message as input, and none of them have failed with this patch, so I guess that's a good thing? :P
> 
> Oh, that is fine :) Thanks for looking. I just mentioned it since you are new-ish. The tests that run as part of GHAs are only a tiny subset of all tests though, therefore there can be tests that fail and you would not notice.
> 
> About the rest, starting to pull a thread somewhere and then noticing that the thread gets longer and longer is a normal thing. Then its up to you to decide whether fixing the isolated issue is worth it or whether more code needs to be reworked.
> 
> Cheers, Thomas

@tstuefe Do you think this should be ok now, since all reliance on [JDK-8292016](https://bugs.openjdk.org/browse/JDK-8292016) has been dropped?

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

PR: https://git.openjdk.org/jdk/pull/9749


More information about the core-libs-dev mailing list