JDK-8276809: java/awt/font/JNICheck/FreeTypeScalerJNICheck.java shows JNI warning on Windows

[MBaesken]

The new test java/awt/font/JNICheck/FreeTypeScalerJNICheck.java introduced with https://bugs.openjdk.java.net/browse/JDK-8269223 adds -Xcheck:jni , and shows on Windows server 2019 the following JNI warning , so the test fails on this Windows version.

stdout: [WARNING in native method: JNI call made without checking exceptions when required to from CallStaticVoidMethodV
at sun.awt.Win32GraphicsEnvironment.initDisplay(java.desktop@23-internal/Native Method)
at sun.awt.Win32GraphicsEnvironment.initDisplayWrapper(java.desktop@23-internal/Win32GraphicsEnvironment.java:95)
at sun.awt.Win32GraphicsEnvironment.(java.desktop@23-internal/Win32GraphicsEnvironment.java:63)
at sun.awt.PlatformGraphicsInfo.createGE(java.desktop@23-internal/PlatformGraphicsInfo.java:34)
at java.awt.GraphicsEnvironment$LocalGE.createGE(java.desktop@23-internal/GraphicsEnvironment.java:93)
at java.awt.GraphicsEnvironment$LocalGE.(java.desktop@23-internal/GraphicsEnvironment.java:84)
at java.awt.GraphicsEnvironment.getLocalGraphicsEnvironment(java.desktop@23-internal/GraphicsEnvironment.java:106)
at FreeTypeScalerJNICheck.runTest(FreeTypeScalerJNICheck.java:53)
at FreeTypeScalerJNICheck.main(FreeTypeScalerJNICheck.java:44)

We better add an exception check to get rid of the JNI warning (and also of the test failure) .

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8276809: java/awt/font/JNICheck/FreeTypeScalerJNICheck.java shows JNI warning on Windows (Bug - P4)

Reviewers

• Ralf Schmelter (@schmelter-sap - Reviewer)
• Thomas Stuefe (@tstuefe - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/17224/head:pull/17224
$ git checkout pull/17224

Update a local copy of the PR:
$ git checkout pull/17224
$ git pull https://git.openjdk.org/jdk.git pull/17224/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 17224

View PR using the GUI difftool:
$ git pr show -t 17224

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/17224.diff

Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back mbaesken! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@MBaesken The following label will be automatically applied to this pull request:

• client

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 00: Full (00759968)

[MBaesken]

Would be nice to get a review, the removed warning message leads to at least -1 failing test in our CI on Windows .

[schmelter-sap]

Looks good to me. Not sure if we really need the logging, since the called method only sets a boolean field.

[openjdk]

@MBaesken This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8276809: java/awt/font/JNICheck/FreeTypeScalerJNICheck.java shows JNI warning on Windows

Reviewed-by: rschmelter, stuefe

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 68 new commits pushed to the master branch:

• eb9e754: 8323065: Unneccesary CodeBlob lookup in CompiledIC::internal_set_ic_destination
• a40d397: 8323110: Eliminate -Wparentheses warnings in ppc code
• 7edd10e: 8321786: SegmentAllocator:allocateFrom(ValueLayout, MemorySegment,ValueLayout,long,long) spec mismatch in exception scenario
• d75d876: 8322806: Eliminate -Wparentheses warnings in aarch64 code
• e442769: 8322754: click JComboBox when dialog about to close causes IllegalComponentStateException
• 3560e68: 8322815: Eliminate -Wparentheses warnings in shenandoah code
• faa9c69: 8322846: Running with -Djdk.tracePinnedThreads set can hang
• ace010b: 8319757: java/nio/channels/DatagramChannel/InterruptibleOrNot.java failed: wrong exception thrown
• be4614e: 8323016: Improve reporting for bad options
• 35a1b77: 8322636: [JVMCI] HotSpotSpeculationLog can be inconsistent across a single compile
• ... and 58 more: https://git.openjdk.org/jdk/compare/a5cf4210cd9c293a9e9bce60dc6d0f08fd838c77...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[MBaesken]

Hi Ralf, thanks for the review !

Any other comments / reviews ?

[tstuefe]

+1. I agree with Ralf, logging is not that important. Possibly just add a (c-runtime) assert. Up to you.

[MBaesken]

Hi Thomas, thanks for the review !

/integrate

[openjdk]

Going to push as commit 71aac7a.
Since your change was applied there have been 69 commits pushed to the master branch:

• 09c6c4f: 8322489: 22-b27: Up to 7% regression in all Footprint3-*-G1/ZGC
• eb9e754: 8323065: Unneccesary CodeBlob lookup in CompiledIC::internal_set_ic_destination
• a40d397: 8323110: Eliminate -Wparentheses warnings in ppc code
• 7edd10e: 8321786: SegmentAllocator:allocateFrom(ValueLayout, MemorySegment,ValueLayout,long,long) spec mismatch in exception scenario
• d75d876: 8322806: Eliminate -Wparentheses warnings in aarch64 code
• e442769: 8322754: click JComboBox when dialog about to close causes IllegalComponentStateException
• 3560e68: 8322815: Eliminate -Wparentheses warnings in shenandoah code
• faa9c69: 8322846: Running with -Djdk.tracePinnedThreads set can hang
• ace010b: 8319757: java/nio/channels/DatagramChannel/InterruptibleOrNot.java failed: wrong exception thrown
• be4614e: 8323016: Improve reporting for bad options
• ... and 59 more: https://git.openjdk.org/jdk/compare/a5cf4210cd9c293a9e9bce60dc6d0f08fd838c77...master

Your commit was automatically rebased without conflicts.

[openjdk]

@MBaesken Pushed as commit 71aac7a.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

[mlbridge]

Mailing list message from Philip Race on client-libs-dev:

I have two concerns with this integrated fix

(1) Not a single developer who works on the client technology approved
this.
Client devs don't go unilaterally pushing fixes in hotspot without
someone from hotspot approving.
Even if JDK has no formal way to? verify that, it is really not
acceptable to have this happen.
And I have raised this before !
https://mail.openjdk.org/pipermail/client-libs-dev/2023-July/014074.html

(2) This fix was proposed before and discussed here
https://mail.openjdk.org/pipermail/client-libs-dev/2021-November/001116.html
where it was requested that it be withdrawn until there is an actual
understanding of the
reason for the exception which has NEVER been seen in our testing here
and once understood
rather than suppressing it needs an actual fix and yet here it is again
in the form previously rejected.

This fix should be backed out. Please send out a PR to do so.

-phil.

On 1/8/24 3:48 AM, Matthias Baesken wrote:

[RealCLanger]

Hi Phil,

first of all, I guess it was an oversight of Matthias to not wait for some client libs representative to have a look.

I agree that the integrated fix is potentially hiding a real issue which should rather be analysed a bit deeper. @MBaesken , can you please take care of the revert and then install a diagnosis patch in our test system that would print the exception and then bail out. That way we could maybe understand more about the actual root cause?

[MBaesken]

Okay I can back it out, will created a JBS issue for that, https://bugs.openjdk.org/browse/JDK-8323330 . However the current JNI warning gives us no real info, just creates noise in our test infrastructure; the ExceptionDescribe should be better : "Prints an exception and a backtrace of the stack to a system error-reporting channel, such as stderr. This is a convenience routine provided for debugging." https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/functions.html

[tstuefe]

Hi Phil,

sorry for this, this was not bad intent, just a plain mistake. It is somewhat difficult to remember the exact review rules per project. Help from Skara would certainly be useful here.

Cheers, Thomas

[MBaesken]

reason for the exception which has NEVER been seen in our testing here and once understood

I checked the new output of the jtr with this change (where ExceptionDescribe is called) but nothing is reported unfortunately.
So I really wonder - is there a exception present that we could try to understand ?

rather than suppressing it needs an actual fix and yet here it is again in the form previously rejected.

Without getting the exception (if there is one?) it is hard to propose a fix; it is not even totally clear if there is need for a fix.