Use new JVM options from JEP 471 and JEP 472 in CI testing#511
Merged
jcflack merged 6 commits intoREL1_6_STABLEfrom Feb 21, 2025
Merged
Use new JVM options from JEP 471 and JEP 472 in CI testing#511jcflack merged 6 commits intoREL1_6_STABLEfrom
jcflack merged 6 commits intoREL1_6_STABLEfrom
Conversation
JEP 472 issues warnings (one day to be errors) for uses of System.loadLibrary or when native methods are first bound according to the default binding process. PL/Java's use of JNI escapes without warnings because the JVM is launched from C and uses RegisterNatives to bind the native methods explicitly, so it does not involve loadLibrary or any use of the default native method matching. On the other hand, the pgjdbc-ng library furnishes netty, which has a use of loadLibrary, and produces warnings from the client side in the CI script. As Node.java now supports both pgjdbc and pgjdbc-ng, switch to pgjdbc in the CI scripts to avoid a nuisance warning.
8a6b1b4 noted that the nearly-identical blobs of jshell in three different CI service configs would be ripe for factoring out into one common version, but left that chore for another day. This may as well be the day.
Since JEP 471 in Java 23, --sun-misc-unsafe-memory-access=deny can be used. Since JEP 472 in Java 24, --illegal-native-access=deny can be used. Both would be best practice, especially in Java 24+ where no policy is available to control access to Unsafe or loadLibrary. Refactor the test script slightly so the Java major version and PostgreSQL major version aren't repeatedly obtained.
None of this is related at all to JEP 472, but some is needed to catch up with changes on the GitHub runners as the CI would otherwise fail. The macos-12 runner is gone, and it was the last macos one that supplied PostgreSQL preinstalled. So rename the workflow from ci-runnerpg to ci-lazypg ... it still uses the preinstalled PG on runners that have one, but now resorts to brew to install it on macos. The macos-13 runner is still Intel-based (and needs PG installed). The macos-14 runner is ARM64-based (and needs PG installed). The runners have some Java versions preinstalled, so check for those and only fetch a JDK if the matrix.java version isn't one of the preinstalled ones. Preinstalled versions can be found by their JAVA_HOME_$VERSION_$ARCH environment variables, except that when $ARCH is ARM64 the environment variable has arm in lowercase. So much for consistency. There are newer versions of the setup-java and checkout actions. The old ones elicited warnings about a deprecated Node.js version. The old setup-java, when running on macos ARM64 (or arm64), would happily install an x86_64 JDK. Which would seem to work fine, apparently running in a compatibility mode, apparently inherited by the C compiler and linker spawned from Maven. The linker would then report baffling errors about not finding symbols in PostgreSQL for x86_64! This is the exact sort of mystery seen and not yet solved in issues #411 and #428. So, running Maven on a JDK that's for the wrong architecture is now a known way to cause such a problem, and worth checking in such a case. Added some ::group:: / ::endgroup:: to make the log output more easily perused.
Also, seems there's no longer a 9 (or 10) JDK version either preinstalled or fetchable for a GitHub runner.
Miscellaneous keeping up with GitHub Actions changes so the JEP472 changes can successfully run.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
JEP 471 adds (from Java 23 on) the possibility to invoke the JVM with
--sun-misc-unsafe-memory-access=deny.JEP 472 adds (from Java 24 on) the possibility to invoke the JVM with
--illegal-native-access=deny.To use both options would be best practice (especially on Java 24+, with the loss of any policy enforcement on access to
UnsafeorloadLibrary), so it is worthwhile to make sure they are used inpljava.vmoptionsfor the CI tests (on the Java versions that support them).Also switch the CI scripts from using the
pgjdbc-ngdriver to usepgjdbcinstead. The-ngdriver usesnetty, which does attempt aloadLibrary. That's on the client side (injshell), so isn't affected by adding the options above topljava.vmoptionson the server side. But the change will save a warning now (and a failure in a laterjshellversion wheredenybecomes the default).A long-overdue refactoring (first shirked in 8a6b1b4) finally factors out the nearly-identical
jshellscripts from the GitHub Actions, AppVeyor, and Travis configurations, into one file they all can share.Several other miscellaneous changes to keep up with the moving target that is GitHub Actions.