On Mon, 29 Jan 2024 07:13:49 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 six additional > commits since the last revision: > > - Move Exception Check to the right place > - Merge branch 'master' into JDK-8323664 > - 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 aivanov (Reviewer). test/jdk/java/awt/font/JNICheck/FreeTypeScalerJNICheck.java line 48: > 46: ProcessBuilder pb = > ProcessTools.createTestJavaProcessBuilder("-Xcheck:jni", > FreeTypeScalerJNICheck.class.getName(), "runtest"); > 47: OutputAnalyzer oa = ProcessTools.executeProcess(pb); > 48: > oa.shouldContain("Done").shouldNotContain("WARNING").shouldNotContain("AWT > Assertion").shouldHaveExitValue(0); Maybe wrap each condition to its own line? Suggestion: oa.shouldContain("Done") .shouldNotContain("WARNING") .shouldNotContain("AWT Assertion") .shouldHaveExitValue(0); This way each verified condition stands out. ------------- PR Review: https://git.openjdk.org/jdk/pull/17404#pullrequestreview-1849728445 PR Review Comment: https://git.openjdk.org/jdk/pull/17404#discussion_r1470188088