Add Add-Opens for java.base/java.lang (JDKSpecifics) #47637
Merged
Conversation
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
quarkus-bot
bot
added
area/core
area/dependencies
|
Tested with for example And also in JVM mode on JDK 24 with #47613: And with Note: This is also being added for JDKs < 24, but it does no harm there. |
gsmet
reviewed
May 1, 2025
| the propertiesCache in com.sun.naming.internal.ResourceManager --> | ||
| <!-- the add-opens java.base/java.lang=ALL-UNNAMED is here for | ||
| org.jboss.JDKSpecific.ThreadAccess.clearThreadLocals() --> | ||
| <argLine>${jacoco.agent.argLine} -Xmx1500m -XX:MaxMetaspaceSize=1500m -Djava.io.tmpdir="${project.build.directory}" ${surefire.argLine.additional} --add-opens java.naming/com.sun.naming.internal=ALL-UNNAMED --add-opens java.base/java.lang=ALL-UNNAMED</argLine> |
There was a problem hiding this comment.
This specific change raises a question: will our users be fine? I would not be terribly excited if they had to tweak their Surefire/Failsafe argLines.
There was a problem hiding this comment.
Do you recall this being the case with 38d9997 ? If not, it looks like it won't impact them.
There was a problem hiding this comment.
FWIW, I haven't seen an actual test failure if this isn't present. It just avoids ugly stack traces on quarkus shut-down. But open to other suggestions as well.
There was a problem hiding this comment.
What I wanted to clarify is: do we have this ugly stacktraces on a standard generated project with standard testing infra or is it fine?
If we do then we probably have an issue and either we find a way to fix it or we will have to document things in the migration guide and maybe include this in the generated template.
I'm not asking you to do the changes but it would be nice to have a status on this so that we can discuss it further if necessary.
There was a problem hiding this comment.
@gsmet Here is what I tried (with the patch in this PR):
$ cat run_quarkus_code_quickstart_quarkus_local.sh
rm -rf code-with-quarkus*
JAVA_HOME=$1
if [ "${JAVA_HOME}_" == "_" ]; then
echo "Usage: bash $0 <path-to-JDK>" 1>&2
exit 1
fi
export JAVA_HOME
curl -O -J "https://code.quarkus.io/d?e=rest&cn=code.quarkus.io"
unzip code-with-quarkus.zip
cd code-with-quarkus
./mvnw -Dquarkus.platform.group-id=io.quarkus -Dquarkus.platform.version=999-SNAPSHOT clean package
$JAVA_HOME/bin/java -jar target/quarkus-app/quarkus-run.jar
And the ugly stacktraces are there when run on JDK 24.0.1 (for example). Output looks like:
inflating: code-with-quarkus/.dockerignore
inflating: code-with-quarkus/mvnw.cmd
inflating: code-with-quarkus/mvnw
WARNING: A restricted method in java.lang.System has been called
WARNING: java.lang.System::load has been called by org.fusesource.jansi.internal.JansiLoader in an unnamed module (file:/home/sgehwolf/.m2/wrapper/dists/apache-maven-3.9.9-bin/33b4b2b4/apache-maven-3.9.9/lib/jansi-2.4.1.jar)
WARNING: Use --enable-native-access=ALL-UNNAMED to avoid a warning for callers in this module
WARNING: Restricted methods will be blocked in a future release unless native access is enabled
WARNING: A terminally deprecated method in sun.misc.Unsafe has been called
WARNING: sun.misc.Unsafe::objectFieldOffset has been called by com.google.common.util.concurrent.AbstractFuture$UnsafeAtomicHelper (file:/home/sgehwolf/.m2/wrapper/dists/apache-maven-3.9.9-bin/33b4b2b4/apache-maven-3.9.9/lib/guava-33.2.1-jre.jar)
WARNING: Please consider reporting this to the maintainers of class com.google.common.util.concurrent.AbstractFuture$UnsafeAtomicHelper
WARNING: sun.misc.Unsafe::objectFieldOffset will be removed in a future release
[INFO] Scanning for projects...
[INFO]
[INFO] ---------------------< org.acme:code-with-quarkus >---------------------
[INFO] Building code-with-quarkus 1.0.0-SNAPSHOT
[INFO] from pom.xml
[INFO] --------------------------------[ jar ]---------------------------------
[INFO]
[INFO] --- clean:3.2.0:clean (default-clean) @ code-with-quarkus ---
[INFO]
[INFO] --- resources:3.3.1:resources (default-resources) @ code-with-quarkus ---
[INFO] Copying 1 resource from src/main/resources to target/classes
[INFO]
[INFO] --- quarkus:999-SNAPSHOT:generate-code (default) @ code-with-quarkus ---
[INFO]
[INFO] --- compiler:3.14.0:compile (default-compile) @ code-with-quarkus ---
[INFO] Recompiling the module because of changed source code.
[INFO] Compiling 1 source file with javac [debug parameters release 21] to target/classes
[INFO]
[INFO] --- quarkus:999-SNAPSHOT:generate-code-tests (default) @ code-with-quarkus ---
[INFO]
[INFO] --- resources:3.3.1:testResources (default-testResources) @ code-with-quarkus ---
[INFO] skip non existing resourceDirectory /home/sgehwolf/Documents/openjdk/quarkus/code-with-quarkus/code-with-quarkus/src/test/resources
[INFO]
[INFO] --- compiler:3.14.0:testCompile (default-testCompile) @ code-with-quarkus ---
[INFO] Recompiling the module because of changed dependency.
[INFO] Compiling 2 source files with javac [debug parameters release 21] to target/test-classes
[INFO]
[INFO] --- surefire:3.5.2:test (default-test) @ code-with-quarkus ---
[INFO] Using auto detected provider org.apache.maven.surefire.junitplatform.JUnitPlatformProvider
[INFO]
[INFO] -------------------------------------------------------
[INFO] T E S T S
[INFO] -------------------------------------------------------
WARNING: A terminally deprecated method in sun.misc.Unsafe has been called
WARNING: sun.misc.Unsafe::allocateMemory has been called by io.netty.util.internal.PlatformDependent0$2 (file:/home/sgehwolf/.m2/repository/io/netty/netty-common/4.1.119.Final/netty-common-4.1.119.Final.jar)
WARNING: Please consider reporting this to the maintainers of class io.netty.util.internal.PlatformDependent0$2
WARNING: sun.misc.Unsafe::allocateMemory will be removed in a future release
[INFO] Running org.acme.GreetingResourceTest
2025-05-06 18:53:50,653 INFO [io.quarkus] (main) code-with-quarkus 1.0.0-SNAPSHOT on JVM (powered by Quarkus 999-SNAPSHOT) started in 1.537s. Listening on: http://localhost:8081
2025-05-06 18:53:50,654 INFO [io.quarkus] (main) Profile test activated.
2025-05-06 18:53:50,655 INFO [io.quarkus] (main) Installed features: [cdi, rest, smallrye-context-propagation, vertx]
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.898 s -- in org.acme.GreetingResourceTest
Exception in thread "vert.x-internal-blocking-1" java.lang.IllegalAccessError: module java.base does not open java.lang to unnamed module @345d053b; to use the thread-local-reset capability on Java 24 or later, use this JVM option: --add-opens java.base/java.lang=ALL-UNNAMED
at org.jboss.threads.JDKSpecific$ThreadAccess.<clinit>(JDKSpecific.java:32)
at org.jboss.threads.ThreadLocalResettingRunnable.run(ThreadLocalResettingRunnable.java:13)
at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
at java.base/java.lang.Thread.run(Thread.java:1447)
Exception in thread "executor-thread-1" java.lang.NoClassDefFoundError: Could not initialize class org.jboss.threads.JDKSpecific$ThreadAccess
at org.jboss.threads.ThreadLocalResettingRunnable.run(ThreadLocalResettingRunnable.java:13)
at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
at java.base/java.lang.Thread.run(Thread.java:1447)
Caused by: java.lang.ExceptionInInitializerError: Exception java.lang.IllegalAccessError: module java.base does not open java.lang to unnamed module @345d053b; to use the thread-local-reset capability on Java 24 or later, use this JVM option: --add-opens java.base/java.lang=ALL-UNNAMED [in thread "vert.x-internal-blocking-1"]
at org.jboss.threads.JDKSpecific$ThreadAccess.<clinit>(JDKSpecific.java:32)
2025-05-06 18:53:51,292 INFO [io.quarkus] (main) code-with-quarkus stopped in 0.017s
... 3 more
[INFO]
[INFO] Results:
[INFO]
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0
[INFO]
[INFO]
[INFO] --- jar:3.4.1:jar (default-jar) @ code-with-quarkus ---
[INFO] Building jar: /home/sgehwolf/Documents/openjdk/quarkus/code-with-quarkus/code-with-quarkus/target/code-with-quarkus-1.0.0-SNAPSHOT.jar
[INFO]
[INFO] --- quarkus:999-SNAPSHOT:build (default) @ code-with-quarkus ---
[INFO] [io.quarkus.deployment.QuarkusAugmentor] Quarkus augmentation completed in 961ms
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 6.904 s
[INFO] Finished at: 2025-05-06T18:53:52+02:00
[INFO] ------------------------------------------------------------------------
WARNING: A terminally deprecated method in sun.misc.Unsafe has been called
WARNING: sun.misc.Unsafe::allocateMemory has been called by io.netty.util.internal.PlatformDependent0$2 (file:/home/sgehwolf/Documents/openjdk/quarkus/code-with-quarkus/code-with-quarkus/target/quarkus-app/lib/main/io.netty.netty-common-4.1.119.Final.jar)
WARNING: Please consider reporting this to the maintainers of class io.netty.util.internal.PlatformDependent0$2
WARNING: sun.misc.Unsafe::allocateMemory will be removed in a future release
__ ____ __ _____ ___ __ ____ ______
--/ __ \/ / / / _ | / _ \/ //_/ / / / __/
-/ /_/ / /_/ / __ |/ , _/ ,< / /_/ /\ \
--\___\_\____/_/ |_/_/|_/_/|_|\____/___/
2025-05-06 18:53:53,199 INFO [io.quarkus] (main) code-with-quarkus 1.0.0-SNAPSHOT on JVM (powered by Quarkus 999-SNAPSHOT) started in 0.478s. Listening on: http://0.0.0.0:8080
2025-05-06 18:53:53,204 INFO [io.quarkus] (main) Profile prod activated.
2025-05-06 18:53:53,204 INFO [io.quarkus] (main) Installed features: [cdi, rest, smallrye-context-propagation, vertx]
^C2025-05-06 18:54:22,057 INFO [io.quarkus] (Shutdown thread) code-with-quarkus stopped in 0.017s
There was a problem hiding this comment.
So my take is that we'd need to update the code templates to include --add-opens java.base/java.lang=ALL-UNNAMED argline for surefire, or add it to the migration guide.
There was a problem hiding this comment.
I've filed #47769 to track this work.
There was a problem hiding this comment.
@dmlloyd I'm a bit worried by us having to add --add-opens java.base/java.lang=ALL-UNNAMED to user apps Surefire configuration. There's no way around it?
There was a problem hiding this comment.
We could always just stop clearing thread locals though. That'd avoid using this class.
This comment has been minimized.
This comment has been minimized.
|
How should we proceed with this? |
Add the equivalent of --add-opens=java.base/java.lang=ALL-UNNAMED to the quarkus runner jars' MANIFEST.MF. The 3.9 update of JBoss Threads removes some Unsafe usages and replaces that by supported JDK APIs. The new code, however, uses field.setAccessible(true), requiring the Add-Opens manifest entry. For the quarkus tests we add the option via the argLine parameter. Closes: quarkusio#47566 (together with the 3.9.1 jboss-threads update)
jerboaa
force-pushed
the
fix_add_opens_JDKSpecifics
branch
from
4647818 to
82b8a53
Compare
Status for workflow
|
|
@gsmet Can we get this merged please, we are not getting any good signal in our CI for JDK 24/25 and risk not seeing issues we need to act on. Happy to follow up with whatever is needed. Thanks! |
jerboaa
commented
May 8, 2025
| the propertiesCache in com.sun.naming.internal.ResourceManager --> | ||
| <!-- the add-opens java.base/java.lang=ALL-UNNAMED is here for | ||
| org.jboss.JDKSpecific.ThreadAccess.clearThreadLocals() --> | ||
| <argLine>${jacoco.agent.argLine} -Xmx1500m -XX:MaxMetaspaceSize=1500m -Djava.io.tmpdir="${project.build.directory}" ${surefire.argLine.additional} --add-opens java.naming/com.sun.naming.internal=ALL-UNNAMED --add-opens java.base/java.lang=ALL-UNNAMED</argLine> |
There was a problem hiding this comment.
I've filed #47769 to track this work.
gsmet
approved these changes
May 8, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Add the equivalent of --add-opens=java.base/java.lang=ALL-UNNAMED to the quarkus runner jars' MANIFEST.MF.
The 3.9 update of JBoss Threads removes some Unsafe usages and replaces that by supported JDK APIs. The new code, however, uses field.setAccessible(true), requiring the Add-Opens manifest entry.
For the quarkus tests we add the option via the argLine parameter.
Closes: #47566 (together with the 3.9.1 jboss-threads update)