Introduce a ModuleOpenBuildItem to let extensions control --add-opens needs for the runtime

[Sanne]

Implementation of what's being discussed on:

• #dev > JVM parameters recommender

Fixes #51434

(This was a draft, updating description)

This is part of a larger plan described on https://github.com/quarkusio/quarkus/discussions/51041 but can be merged independently.

It should be followed up some further polishing to be tracked on https://github.com/orgs/quarkusio/projects/59/views/1 - but I consider this a good incremental improvement which doesn't strictly require any follow-up.

Most notably this will avoid all warnings related to module usage, and reconfigure the module system fully transparently w/o users needing to apply any changes in most cases. When we can't fully automate something, an actionable warning should be visible.

[Sanne]

Example usage by Netty cc/ @franz1981

• https://github.com/quarkusio/quarkus/commit/0c050b99e4a409a57e09bf838afb528b12736926

[zakkak]

Just a note for consistency. There is JPMSExportBuildItem which we use in native-mode. IMHO we should use similar names for both the "export" and the "open" builditems, so I suggest using JPMSOpenBuildItem or renaming JPMSExportBuildItem accordingly.

[Sanne]

thanks @zakkak ! I see the SecurityProcessor also produces some JPMSExportBuildItem(s), looks like related to Fips:

• https://github.com/quarkusio/quarkus/blob/main/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/SecurityProcessor.java#L449-L452

Shouldn't these also be applied to the Jar Manifest (and any other launcher we'll support) ?

[dmlloyd]

Just a note for consistency. There is JPMSExportBuildItem which we use in native-mode. IMHO we should use similar names for both the "export" and the "open" builditems, so I suggest using JPMSOpenBuildItem or renaming JPMSExportBuildItem accordingly.

This one needs to be revisited because it has several problems:

• It is clearly a global concept, so it should not be in the nativeimage subpackage.
• It includes GraalVM version information. This is just flat-out wrong; any version range predicate should be evaluated by the step producing the item. If the version is outside of the range, the item should not be produced.
• There is no destination module name.

IMO it might be easiest to deprecate JPMSExportBuildItem for removal and go to unified items like what I described.

[Sanne]

@dmlloyd agreed, I noticed the same limitations - clearly JPMSExportBuildItem was designed specifically for native-image. It begs for us to define some sort of "target JVM filter" that would address version ranges and platforms in a more generic way.

But before thinking of providing a more general solution I'd like to check if there's any need for this - as far as I can see that Fips Security Processor is the only user for "add exports" - so let's hear if that should be applied to JVM mode as well.

[dmlloyd]

But before thinking of providing a more general solution I'd like to check if there's any need for this - as far as I can see that Fips Security Processor is the only user for "add exports" - so let's hear if that should be applied to JVM mode as well.

The modular build requires at least one add-exports, and the current loom experiments also require one or maybe two add-exports.

"WAGNI" - We Are Going To Need It!

[zakkak]

JPMSExportBuildItem is also used in:

• https://github.com/quarkusio/quarkus/blob/52c39d36492111c4aa09ace9d166ce451a1bc028/extensions/jdbc/jdbc-db2/deployment/src/main/java/io/quarkus/jdbc/db2/deployment/JDBCDB2Processor.java#L177
• https://github.com/quarkusio/quarkus/blob/52c39d36492111c4aa09ace9d166ce451a1bc028/core/deployment/src/main/java/io/quarkus/deployment/steps/NativeImageFeatureStep.java#L61-L69 (this is graalvm-related)

I agree with @dmlloyd on "WAGNI" :)

Shouldn't these also be applied to the Jar Manifest (and any other launcher we'll support) ?

It really depends on whether the code paths triggering the issue (i.e. accessing the encapsulated code) are reached at runtime or not. We have no tests triggering it in JVM-mode AFAIK and IIRC no-one digged deeper to see whether we could prune these code paths in native-mode.

[dmlloyd]

I'm not sure I'd depend on a test to determine whether something is needed. But maybe it's worth trying to craft one to trigger the issue?

[Sanne]

"WAGNI" - We Are Going To Need It!

very well, I know I can trust you on such feelings :)

This particular POC isn't linked to any specific issue or strategy yet (I was just exploring options), I guess I'll include the need for add-exports to be addressed by the same "thing" which is emerging here.

[gsmet]

@Sanne do you plan to work further on this? We are getting more and more questions about our Java 25 support and we really need to get rid of the scary warning coming from JBoss Threads.

[Sanne]

@Sanne do you plan to work further on this?

Definitely.

We are getting more and more questions about our Java 25 support and we really need to get rid of the scary warning coming from JBoss Threads.

I'm wondering about this - even while this PR is work in progress, the built Jars already do have the right flags for JBoss Threads? This PR is exploring how to make such an approach more general, and a consistent solution for other extensions should they have a similar need.

Could you share details of such reports? I suspect they might relate to other running modes - We'd need to think how to address those as well. I need to track more of such cases.

[Sanne]

I'm also having a look into:

• https://github.com/quarkiverse/quarkus-groovy/issues/256
• https://github.com/quarkiverse/quarkus-flow/issues/8 as they both suggest of problems about adding such flags - at very least I want to understand such implications better.

[jerboaa]

@Sanne do you plan to work further on this? We are getting more and more questions about our Java 25 support and we really need to get rid of the scary warning coming from JBoss Threads.

Right, the fix is in quarkus main. The warnings happen for newly generated code starters, I think. This is tracked in https://github.com/quarkusio/quarkus/issues/47769

[gsmet]

OK, I misunderstood then, mostly because we still have this issue in dev mode, which is expected.

I'll go fix the issues in dev mode and Surefire and see if I can come up with an OpenRewrite recipe for existing projects.

[Sanne]

@gsmet I'd be curious to hear what you'd plan to do for dev mode? I honestly don't know how we could solve it - in the generic form. Which also gives me pause on this particular POC - I'd not be happy to expose a generic API to define such needs if we can't honour them consistently.

[dmlloyd]

I think it wouldn't affect the POC. The same build items could be interpreted during dev mode to reflectively invoke additional addOpens, provided we open java.base/java.lang.module (IIRC).

[gsmet]

I have to dig more with dev mode - I have a quick and dirty patch but I haven't checked if we could consume the build items - but I think we have to accept it won't be perfect: typically for Surefire, we will have to add the add-opens in the POM file and they will have to be hardcoded.

[Sanne]

typically for Surefire, we will have to add the add-opens in the POM file and they will have to be hardcoded.

Right - all good to do so for the short time, but it bothers me greatly that we won't be having more flexibility, e.g. as mentioned some extensions really do require additional flags. Ideas very welcome! (by anyone)

I'm inclined to add a "check" phase in bootstrap, so at least we can hint the user "Hey you really should be launching this with -XYZ"

[Sanne]

I've been playing with an experiment (out of tree) in which I'm using an agent to dynamically add any adds-opens that we might need.

It seems to work! We can automatically attach the agent to ourselves and adjust module configurations as we wish. The drawback is we'll need -XX:+EnableDynamicAgentLoading to avoid other warnings, but at least that's a consistent set of options which we can hard code in tool launchers.

So this allows to enable additional opens/exports as needed, as we run any extension's deployment processors, we can apply them on the fly w/o interrupting the user's flow.

Also this option would only be for testing and live-reload coding scenarios - for production mode we don't need the agent and use the Jar manifest still.

I plan to try retro-fitting this prototype into a Quarkus PR next - unless there are fundamental objections to the approach? @dmlloyd @gsmet @geoand please let me know if you think I should not go this route :)

Incidentally, I'm using Bytebuddy. I appreciate it's not ideal to have yet-another bytecode manipulation tool in core, but it makes such a thing very easy to develop.

[dmlloyd]

If you're enabling remote agents, you have to supply an argument; if you're doing that then what's the advantage over opening a package from java.base instead? Don't agents incur a performance penalty?

[Sanne]

If you're enabling remote agents, you have to supply an argument; if you're doing that then what's the advantage over opening a package from java.base instead? Don't agents incur a performance penalty?

There are no agent arguments. We register a single instrumentation agent once, to which I can then get a reference and use this to redefine the modules as needed, programmatically from Quarkus code.

[dmlloyd]

You can programmatically add opens and exports by opening java.base as well, without requiring an agent.

[Sanne]

You can programmatically add opens and exports by opening java.base as well, without requiring an agent.

That sounds promising as well. So for live-reload and tests we could require a single -add-opens on java.base , or perhaps automatically set this via the manifest of our launcher, then allow the build system to dynamically relax further restrictions as needed?

Do you have a pointer on how that would look like?

N.B. that the agent solution would work even if people forget to add the basic opens. We can literally self-load and patch anything, but it will result in a warning being printed (and this will likely no longer work on a future JVM version).

[dmlloyd]

You should be able to call either java.lang.Module#implAddOpens(java.lang.String, java.lang.Module) via method handle, or jdk.internal.module.Modules#addOpens via method handle. The first option requires --add-opens java.base/java.lang=ALL-UNNAMED, while the second option can be accomplished with --add-exports java.base/jdk.internal.module=ALL-UNNAMED since it is public.

[gsmet]

Can't say I'm excited about requiring an agent. Not sure what others think but that's something I would prefer to avoid.

[geoand]

Not the best thing in the world, but FWIW, we already have one added when launching dev-mode 😉

[jerboaa]

Why would -XX:+EnableDynamicAgentLoading be needed? It's an option that is bound to be removed in later JDK releases. So depending on it would kick the tin just a little bit down the road. If an agent is needed, why not use the -javaagent command line option?

[geoand]

If an agent is needed, why not use the -javaagent command line option?

Especially as we already attach an agent for dev-mode (the class change agents)

[gsmet]

It would indeed be transparent in dev mode as we launch the JVM ourself. But it wouldn't in tests run by Surefire. We would have to contribute the agent to the <argLine>.

[jerboaa]

It would indeed be transparent in dev mode as we launch the JVM ourself. But it wouldn't in tests run by Surefire. We would have to contribute the agent to the <argLine>.

Wouldn't the same be true for -XX:+EnableDynamicAgentLoading? If we need to specify that anyway, the better choice would be to use -javaagent directly.