On Thu, 25 Jan 2024 06:36:36 GMT, Christoph Langer <clan...@openjdk.org> wrote:

>> This picks up fixing the issue of 
>> [JDK-8276809](https://bugs.openjdk.org/browse/JDK-8276809) again. A fix had 
>> been integrated with #17224 but @prrace had concerns and so it was backed 
>> out.
>> 
>> I have now spent quite some thoughts into the problem and end up with the 
>> [initial 
>> commit](https://github.com/openjdk/jdk/pull/6306/commits/5d18a76cb967e9ede6394cbd6c28bb61facf785c)
>>  of #6306 as the most elegant and least intrusive solution.
>> 
>> Why is this?
>> 
>> The JNI warning we observe in the test is:
>> `[WARNING in native method: JNI call made without checking exceptions when 
>> required to from CallStaticVoidMethodV
>>      at 
>> sun.awt.Win32GraphicsEnvironment.initDisplay(java.desktop@22.0.1-internal/Native
>>  Method)
>>      at 
>> sun.awt.Win32GraphicsEnvironment.initDisplayWrapper(java.desktop@22.0.1-internal/Win32GraphicsEnvironment.java:95)
>>      at 
>> sun.awt.Win32GraphicsEnvironment.<clinit>(java.desktop@22.0.1-internal/Win32GraphicsEnvironment.java:63)
>>         ...
>>         at FreeTypeScalerJNICheck.runTest(FreeTypeScalerJNICheck.java:53)
>>      at FreeTypeScalerJNICheck.main(FreeTypeScalerJNICheck.java:44)`
>> 
>> This happens because obviously the test FreeTypeScalerJNICheck runs with 
>> `-Xcheck:jni` and in the scenario where we're observing the warning, a 
>> missing exception check for the JNI call to 
>> `sun.awt.Win32GraphicsEnvironment::dwmCompositionChanged` at 
>> awt_Win32GraphicsEnv.cpp#L129
>> https://github.com/openjdk/jdk/blob/c5e72450966ad50d57a8d22e9d634bfcb319aee9/src/java.desktop/windows/native/libawt/windows/awt_Win32GraphicsEnv.cpp#L129
>>  is airing up. Omitting the exception check would not be a problem if it 
>> could be guaranteed that after this call no other JNI->Java call was being 
>> made. But seemingly in this very particular configuration on some of our 
>> Windows servers, there must be JNI->Java calls that follow the call to 
>> `sun.awt.Win32GraphicsEnvironment::dwmCompositionChanged`, likely from the 
>> subsequent call to 
>> [initScreens](https://github.com/openjdk/jdk/blob/c5e72450966ad50d57a8d22e9d634bfcb319aee9/src/java.desktop/windows/native/libawt/windows/awt_Win32GraphicsEnv.cpp#L149)
>>  in `sun.awt.Win32GraphicsEnvironment::initDisplay`. 
>> https://github.com/openjdk/jdk/blob/8b6293f6bfb7b7628c6604e6c44401fc96d85cf4/src/java.desktop/windows/native/libawt/windows/awt_Win32GraphicsEnv.cpp#L141
>>  Maybe the usual control flow would call the wrapping native method 
>> `DWMIsComposit
 ionEnabled()` from somewhere else initially such that the initialization of 
`Win32GraphicsEnvironment` would not go ...
>
> Christoph Langer has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains four additional 
> commits since the last revision:
> 
>  - Changes to assertion function and test as discussed
>  - Revert "JDK-8323664"
>    
>    This reverts commit 32128744252d75104e0d19f5eb701ffdc7b3d417.
>  - Merge branch 'master' into JDK-8323664
>  - JDK-8323664

Marked as reviewed by prr (Reviewer).

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

PR Review: https://git.openjdk.org/jdk/pull/17404#pullrequestreview-1846478583

Reply via email to