Skip to content
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

Avoid java.management dependency when JMX is disabled #2775

Merged
merged 3 commits into from
Jul 29, 2024
Merged

Conversation

vy
Copy link
Member

@vy vy commented Jul 28, 2024

Fixes #2774 by placing code depending on java.management behind a java.management-free check.

@vy vy added the jpms Affects Java module (JPMS) integration label Jul 28, 2024
@vy vy added this to the 2.24.0 milestone Jul 28, 2024
@vy vy requested a review from ppkarwasz July 28, 2024 09:11
@vy vy self-assigned this Jul 28, 2024
@vy vy added the bug Incorrect, unexpected, or unintended behavior of existing code label Jul 28, 2024
Copy link
Contributor

@ppkarwasz ppkarwasz left a comment

Choose a reason for hiding this comment

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

The generated module-info.class still contains the following non optional Java modules:

requires java.base transitive
requires java.compiler transitive
requires java.desktop
requires java.management transitive
requires java.rmi
requires java.xml transitive

We need to mark java.management as static and non-transitive in the BND configuration.

This might cause a couple of problems:

  • As far as I know static modules are not loaded by the JVM.They will only be available at runtime if another modules requires them non-"static"ally. If log4j2.jmxDisabled is set to false a JPMS application might still not support JMX. We should test it and document it.
  • The AsyncLoggerDisruptor and AsyncLoggerConfigDisruptor classes require RingBufferAdmin. They can fail if java.management is not available.

@vy vy merged commit 1e2e920 into 2.x Jul 29, 2024
9 checks passed
@vy vy deleted the fix/2.x/jmx-mod-dep branch July 29, 2024 07:47
@ppkarwasz
Copy link
Contributor

I ran a couple of tests on a simple JLink project to find out that:

  • the HotSpot JVM always loads java.management if it is present (the module provides an implementation of a LoginModule service that is consumed by java.base), so setting java.management as static should not cause problems to existing users: the module will be present at runtime anyway.
  • AsyncLoggerDisruptor and AsyncLoggerConfigDisruptor work correctly even if java.management is missing, so there is nothing to document there.

I think, however, that we should document that setting log4j2.jmxDisabled to false requires JPMS users to add java.management to their module dependencies, otherwise JLink projects will fail to include it.

@ppkarwasz ppkarwasz removed this from the 2.24.0 milestone Aug 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Incorrect, unexpected, or unintended behavior of existing code jpms Affects Java module (JPMS) integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

log4j2.disableJmx and/or log4j2.disable.jmx seem not to work
2 participants