On Fri, 9 Feb 2024 14:20:02 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:
>> Perhaps what we should do here is
>> if (ExceptionCheck) {
>> ExceptionDescribe
>> ExceptionClear
>> }
>> So someone can "see" [yes, this means it isn't propagated but we've printed
>> it and we have the assert coming up anyway] the text of the original
>> exception, and the debugging code is safe to make the calls it wants.
>>
>> The alternatives are that the debugging code in the case of
>> ExceptionCheck==TRUE just do what it takes to silence the JNI warnings ,
>> assuming that TRUE is never not a possibility, so no real problem, but I
>> don't know see how we can be sure about that for ALL callers of this assert
>> code. (BTW I wonder if the reason the current code didn't do as expected is
>> because ExceptionCheck isn't doing what we expect, but I don't see how), or
>> alternative number 2 is that the debug code simply bails in the face of a
>> pending exception, ie
>> if (ExceptionCheck) {
>> return;
>> }
>
> As far as I can see, the real problem is that `DWMIsCompositionEnabled` calls
> a Java method and does not check if an exception occurred. It should do it
> according to the JNI specification.
>
> I can assume `initScreens(env)` does not call JNI methods, therefore no JNI
> warning is produced in the regular code flow where no assertions fail.
> assuming that TRUE is never not a possibility, so no real problem, but I
> don't know see how we can be sure about that for ALL callers of this assert
> code.
We can never be sure about it, even though I tend to believe exceptions are
rare.
Essentially, any upcall into Java followed by an assertion will lead to this
JNI warning because the assertion handler also upcalls into Java.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17404#discussion_r1484389986
