On Wed, 31 Jan 2024 06:12:28 GMT, Christoph Langer <clan...@openjdk.org> wrote:
>> src/java.desktop/windows/native/libawt/windows/awt_Debug.cpp line 158:
>>
>>> 156: // be on the safe side and avoid JNI warnings by calling
>>> ExceptionCheck
>>> 157: // an accumulated exception is not cleared
>>> 158: env->ExceptionCheck();
>>
>> So, this doesn't actually do anything but avoids JNI warnings, does it?
>>
>> `AwtDebugSupport::AssertCallback` can be called *at any time*, hence calling
>> JNI functions here is unsafe. Adding `env->ExceptionCheck()` doesn't change
>> anything.
>
> Yes. However, it's "only" in the assertion callback that only exists in debug
> VMs. And an original exception isn't lost.
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;
}
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17404#discussion_r1483820054
