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

Netty PlatformDependent0.<init> uses DirectByteBuffer reflectively #37628

Conversation

zakkak
Copy link
Contributor

@zakkak zakkak commented Dec 8, 2023

Since we moved io.netty.util.internal.PlatformDependent's and io.netty.util.internal.PlatformDependent0' initialization at runtime we need to register java.nio.DirectByteBuffer's constructors for reflective access to avoid miss-configurations due to not being able to access the constructor in
https://github.com/netty/netty/blob/5db037beedca8aa5b6a486fa515cc6af013ced74/common/src/main/java/io/netty/util/internal/PlatformDependent0.java#L290-L292

More classes and methods might need to be registered as discussed in #37626

Closes Karm/mandrel-integration-tests#236

Note this is not a complete fix for #37626, it only partially solves the problem and unblocks our (mandrel team) CI.

Since we moved `io.netty.util.internal.PlatformDependent`'s and
`io.netty.util.internal.PlatformDependent0`' initialization at runtime
we need to register `java.nio.DirectByteBuffer`'s constructors for
reflective access to avoid miss-configurations due to not being able to
access the constructor in
https://github.com/netty/netty/blob/5db037beedca8aa5b6a486fa515cc6af013ced74/common/src/main/java/io/netty/util/internal/PlatformDependent0.java#L290-L292

More classes and methods need to be registered as discussed in
quarkusio#37626

Closes Karm/mandrel-integration-tests#236
@zakkak
Copy link
Contributor Author

zakkak commented Dec 8, 2023

FWIW setting io.netty.tryReflectionSetAccessible=false at native build time seems to work around the issue.

See Karm/mandrel-integration-tests#236 (comment)

Interestingly DirectByteBuffer's constructor is not reachable in JVM-mode, but it doesn't seem to cause any issues:

2023-12-09 00:17:15,143 TRACE [io.net.uti.int.PlatformDependent0] (Thread-3) direct buffer constructor: unavailable: java.lang.UnsupportedOperationException: Reflective setAccessible(true) disabled
	at io.netty.util.internal.ReflectionUtil.trySetAccessible(ReflectionUtil.java:31)
	at io.netty.util.internal.PlatformDependent0$5.run(PlatformDependent0.java:293)
	at java.base/java.security.AccessController.doPrivileged(AccessController.java:319)
	at io.netty.util.internal.PlatformDependent0.<clinit>(PlatformDependent0.java:286)
	at io.netty.util.internal.PlatformDependent.isAndroid(PlatformDependent.java:331)
	at io.netty.util.internal.PlatformDependent.<clinit>(PlatformDependent.java:86)
	at io.netty.channel.DefaultChannelId.defaultProcessId(DefaultChannelId.java:177)
	at io.netty.channel.DefaultChannelId.<clinit>(DefaultChannelId.java:77)
	at io.quarkus.netty.runtime.NettyRecorder$1.run(NettyRecorder.java:28)
	at java.base/java.lang.Thread.run(Thread.java:1583)

and the reason is that "Reflective setAccessible(true)" is disabled, which is what I would expect io.netty.tryReflectionSetAccessible=false to do, yet when setting it in native mode it results in the exactly the opposite behavior :/

@zakkak zakkak marked this pull request as draft December 8, 2023 22:26
@zakkak
Copy link
Contributor Author

zakkak commented Dec 8, 2023

Switching to draft since I need to better understand what's going on.

zakkak added a commit to zakkak/quarkus that referenced this pull request Dec 8, 2023
In quarkusio#37347 we moved
`PlatformDependent` and `PlatformDependent0` classes to
run-time-initialization as they are platform-dependent and thus need to
be initialized on the target platform. It looks like there are more
classes like these that need to be runtime initialized as they
transitively rely on platform dependent values.

Closes Karm/mandrel-integration-tests#236

Supersedes quarkusio#37628
@zakkak zakkak closed this Dec 8, 2023
@quarkus-bot quarkus-bot bot added the triage/invalid This doesn't seem right label Dec 8, 2023
@zakkak
Copy link
Contributor Author

zakkak commented Dec 8, 2023

Superseded by #37633

zakkak added a commit to zakkak/quarkus that referenced this pull request Dec 11, 2023
In quarkusio#37347 we moved
`PlatformDependent` and `PlatformDependent0` classes to
run-time-initialization as they are platform-dependent and thus need to
be initialized on the target platform. It looks like there are more
classes like these that need to be runtime initialized as they
transitively rely on platform dependent values.

Closes Karm/mandrel-integration-tests#236

Supersedes quarkusio#37628
@zakkak zakkak deleted the 2023-12-08-fix-netty-platform-dependent-run-time-init branch January 11, 2024 09:30
gsmet pushed a commit to gsmet/quarkus that referenced this pull request Jan 15, 2024
In quarkusio#37347 we moved
`PlatformDependent` and `PlatformDependent0` classes to
run-time-initialization as they are platform-dependent and thus need to
be initialized on the target platform. It looks like there are more
classes like these that need to be runtime initialized as they
transitively rely on platform dependent values.

Closes Karm/mandrel-integration-tests#236

Supersedes quarkusio#37628

(cherry picked from commit 72d7b79)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JFRTest#jfrPerfTest fails with Quarkus main
1 participant