Runtime (re)initialize Netty's PlatformDependent classes

[zakkak]

Closes #17839

[cescoffier]

I'm surprised it is so simple. Pretty sure @gsmet and I tried that already.

Obviously, GraalVM has changed a lot in between.

[cescoffier]

Actually, I use the same trick for the array offset issue.

[zakkak]

I'm surprised it is so simple. Pretty sure @gsmet and I tried that already.

Yes, it was not that simple before. We needed to also disable Unsafe which turned out to have performance issues (see #31016 and #32289). Thanks to @franz1981's work in netty/netty#13643 we no longer need to disable Unsafe for the re-initialization making things pretty simple.

[gsmet]

The error is unrelated: 2023-11-28T09:01:15.9218680Z [INFO] [io.quarkus.deployment.pkg.steps.NativeImageBuildContainerRunner] Checking status of builder image 'quay.io/quarkus/ubi-quarkus-mandrel-builder-image:jdk-21' 2023-11-28T09:01:30.9684851Z Error response from daemon: Get "https://quay.io/v2/": net/http: request canceled (Client.Timeout exceeded while awaiting headers).

[gsmet]

Nice it has become so simple \o/.

[gsmet]

@zakkak actually, I think it doesn't work.

We had these failures lately:

2023-11-29 09:30:46,719 INFO  [io.grp.net.Net.connections] (vert.x-eventloop-thread-0) Transport failed: java.lang.NullPointerException
	at io.netty.util.internal.PlatformDependent0.getLong(PlatformDependent0.java:650)
	at io.netty.util.internal.PlatformDependent0.directBufferAddress(PlatformDependent0.java:625)
	at io.netty.util.internal.PlatformDependent.directBufferAddress(PlatformDependent.java:525)
	at io.netty.buffer.PooledUnsafeDirectByteBuf.initMemoryAddress(PooledUnsafeDirectByteBuf.java:64)
	at io.netty.buffer.PooledUnsafeDirectByteBuf.init(PooledUnsafeDirectByteBuf.java:54)
	at io.netty.buffer.PoolChunk.initBufWithSubpage(PoolChunk.java:615)
	at io.netty.buffer.PoolChunk.initBuf(PoolChunk.java:597)
	at io.netty.buffer.PoolChunk.allocate(PoolChunk.java:354)
	at io.netty.buffer.PoolArena.allocateNormal(PoolArena.java:216)
	at io.netty.buffer.PoolArena.tcacheAllocateSmall(PoolArena.java:180)
	at io.netty.buffer.PoolArena.allocate(PoolArena.java:137)
	at io.netty.buffer.PoolArena.allocate(PoolArena.java:129)

and I'm pretty sure they are related.

I will try to revert and see how it goes.

You can see the details of the failures here for instance: https://github.com/gsmet/quarkus/actions/runs/7023535365#summary-19117391342

[franz1981]

I haven't followed the discussion, but that stacktrace means that UNSAFE isn't populated at https://github.com/netty/netty/blob/84a13a92e1d757965438dc41a5611b19dd47ec44/common/src/main/java/io/netty/util/internal/PlatformDependent0.java#L650

enabling (debug) logging when PlatformDependent0 static init run could be helpful (maybe), because when it fail in some of the steps of it (eg to find a the NIO direct ByteBuffer's address field and others, Unsafe::copyMemory etc etc), it gives up on using it as a whole.

[jerboaa]

@gsmet FWIW, we see similar failures in our CI.

See: https://github.com/graalvm/mandrel/actions/runs/7026828114/job/19121127385#step:12:906

[zakkak]

It indeed seems related, but how come the CI didn't catch it?

I will try to reproduce locally to better understand what's going on.

[gsmet]

@zakkak I think it did catch it but as we had other issues related to quay being unresponsive we probably missed it.

(I contacted the people at quay to see what's going there)

[zakkak]

@zakkak I think it did catch it but as we had other issues related to quay being unresponsive we probably missed it.

it looks like the test didn't run (although it was built), see https://github.com/quarkusio/quarkus/actions/runs/7015924713/job/19093162099#step:14:430 It doesn't look llie a quay thing to me, but I also don't understand what happened and it didn't run.

Anyhow back on the actual issue, it looks like netty sees -Dio.netty.noUnsafe: true at runtime which seems to be set in

quarkus/extensions/infinispan-client/deployment/src/main/java/io/quarkus/infinispan/client/deployment/InfinispanClientProcessor.java

Line 208 in d7842f9

systemProperties.produce(new SystemPropertyBuildItem("io.netty.noUnsafe", "true"));

which was added in Quarkus 0.8.0 and I can't see from the PR or commit message why so I will toy with it.

[gsmet]

Hum, I wonder if we have a problem with the build cache.

[zakkak]

diff --git a/extensions/infinispan-client/deployment/src/main/java/io/quarkus/infinispan/client/deployment/InfinispanClientProcessor.java b/extensions/infinispan-client/deployment/src/main/java/io/quarkus/infinispan/client/deployment/InfinispanClientProcessor.java
index 58201a89f0b..7e11774606f 100644
--- a/extensions/infinispan-client/deployment/src/main/java/io/quarkus/infinispan/client/deployment/InfinispanClientProcessor.java
+++ b/extensions/infinispan-client/deployment/src/main/java/io/quarkus/infinispan/client/deployment/InfinispanClientProcessor.java
@@ -205,7 +205,6 @@ InfinispanPropertiesBuildItem setup(ApplicationArchivesBuildItem applicationArch
         additionalBeans.produce(AdditionalBeanBuildItem.builder().addBeanClass(InfinispanClientName.class).build());
         additionalBeans.produce(AdditionalBeanBuildItem.builder().addBeanClass(Remote.class).build());
 
-        systemProperties.produce(new SystemPropertyBuildItem("io.netty.noUnsafe", "true"));
         hotDeployment
                 .produce(new HotDeploymentWatchedFileBuildItem(META_INF + File.separator + DEFAULT_HOTROD_CLIENT_PROPERTIES));
 

Seems to resolve the issue. Not sure yet if it causes any other issues though.
Being tested in https://github.com/zakkak/quarkus/actions/runs/7032713931

I will open a PR if things look good.

[gsmet]

@karesti @wburns do you remember why Infinispan is setting -Dio.netty.noUnsafe=true in the Quarkus extension?

If there's no good reason, we will probably drop it.

[wburns]

@karesti @wburns do you remember why Infinispan is setting -Dio.netty.noUnsafe=true in the Quarkus extension?

If there's no good reason, we will probably drop it.

At the time I believe it was just because we didn't support UNSAFE in netty so we were using the property to "hint" to not initialize things related to it. As long as Netty native code is working now, there is no reason to have the property.

[gsmet]

Thanks @wburns !