Skip to content

8281089: JavaFX built with VS2019 and jlinked into JDK 11.x fails to start #734

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

kevinrushforth
Copy link
Member

@kevinrushforth kevinrushforth commented Feb 16, 2022

The javafx.graphics module has a number of native libraries, including the redistributable Microsoft DLLs on Windows. When running an application using the standalone JavaFX SDK, we load all of the needed DLLs from the SDK bin directory and everything works as expected.

When creating the jmods, we currently exclude the Microsoft DLLs from javafx.graphics.jmod to avoid the problem described in JDK-8207015 where jlink complains about two modules (java.base and javafx.graphics) deliver the same file(s). That works as long as the set of Microsoft DLLs delivered by the JDK is from the same or newer version of Microsoft Visual Studio. This means that you can't take, for example, JDK 11.0.x and run the latest version of JavaFX, since JDK 11.x uses VS 2017 and JavaFX 15 (and later) uses VS 2019.

Up until now this has just been a minor bug, but we are now at the point where we need to update to VS 2019 for building JavaFX 11.0.15, which means that a jlinked JDK 11.0.X + JavaFX 11.0.15 will no longer work.

This fix is to add the Microsoft DLLs back into javafx.graphics.jmod (i.e., stop excluding them at build time), but deliver them in bin/javafx so they don't conflict with the ones delivered by the JDK.

The changes are:

  1. build.gradle - When creating the jmods on Windows, copy all of the DLLs to a javafx sub-directory
  2. NativeLibLoader.java - in the case of JavaFX modules jlinked into the Java runtime, load the libraries first from ${java.home} rather than always falling back to System::loadLibrary. Most of the changes are just simple refactoring. The additional logic is in the new libDirForJRT method.

When running using the SDK or the modules on maven central, there is no change in behavior.

I've tested this on all platforms, including a test of a custom JDK 11.x created with jlink. On a Windows I tested it on a system where a key VS 2019 runtime library was not present in C:\Windows\System32 and it now runs.


Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed

Issue

  • JDK-8281089: JavaFX built with VS2019 and jlinked into JDK 11.x fails to start

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jfx pull/734/head:pull/734
$ git checkout pull/734

Update a local copy of the PR:
$ git checkout pull/734
$ git pull https://git.openjdk.java.net/jfx pull/734/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 734

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jfx/pull/734.diff

@bridgekeeper
Copy link

bridgekeeper bot commented Feb 16, 2022

👋 Welcome back kcr! 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 openjdk bot added the rfr Ready for review label Feb 16, 2022
@mlbridge
Copy link

mlbridge bot commented Feb 16, 2022

Webrevs

@kevinrushforth
Copy link
Member Author

/reviewers 2

@openjdk
Copy link

openjdk bot commented Feb 16, 2022

@kevinrushforth
The number of required reviews for this PR is now set to 2 (with at least 1 of role reviewers).

Copy link
Collaborator

@tiainen tiainen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've created a sample that generates a jlinked runtime for JDK 11 and OpenJFX 19. I verified that it fails with 19-ea+3 and succeeds with this patch.

@openjdk
Copy link

openjdk bot commented Feb 28, 2022

@kevinrushforth 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:

8281089: JavaFX built with VS2019 and jlinked into JDK 11.x fails to start

Reviewed-by: arapte, sykora

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 5 new commits pushed to the master branch:

  • adf1da4: 8279228: Leak in ScrollPaneSkin, related to touch events
  • 6f201f7: 8282099: Cherry-pick WebKit 613.1 stabilization fixes (2)
  • a0bb545: 8281953: NullPointer in InputMethod components in JFXPanel
  • 7396396: 8282134: Certain regex can cause a JS trap in WebView
  • 418d343: 8281711: Cherry-pick WebKit 613.1 stabilization fixes

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
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.

@openjdk openjdk bot added the ready Ready to be integrated label Feb 28, 2022
@kevinrushforth
Copy link
Member Author

/integrate

@openjdk
Copy link

openjdk bot commented Feb 28, 2022

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

  • adf1da4: 8279228: Leak in ScrollPaneSkin, related to touch events
  • 6f201f7: 8282099: Cherry-pick WebKit 613.1 stabilization fixes (2)
  • a0bb545: 8281953: NullPointer in InputMethod components in JFXPanel
  • 7396396: 8282134: Certain regex can cause a JS trap in WebView
  • 418d343: 8281711: Cherry-pick WebKit 613.1 stabilization fixes

Your commit was automatically rebased without conflicts.

@openjdk openjdk bot added the integrated Pull request has been integrated label Feb 28, 2022
@openjdk openjdk bot closed this Feb 28, 2022
@openjdk openjdk bot removed ready Ready to be integrated rfr Ready for review labels Feb 28, 2022
@openjdk
Copy link

openjdk bot commented Feb 28, 2022

@kevinrushforth Pushed as commit e74cbe8.

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

@kevinrushforth kevinrushforth deleted the 8281089-jlink-vs2019 branch February 28, 2022 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integrated Pull request has been integrated
Development

Successfully merging this pull request may close these issues.

3 participants