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

Add metadata for Netty SelectorProviderUtil #382

Merged
merged 2 commits into from
Jan 23, 2024
Merged

Conversation

czp3009
Copy link
Contributor

@czp3009 czp3009 commented Sep 6, 2023

What does this PR do?

Add reflection hints for Netty SelectorProviderUtil

Netty code in io.netty.channel.socket.nio.SelectorProviderUtil:

@SuppressJava6Requirement(reason = "Usage guarded by java version check")
static Method findOpenMethod(String methodName) {
    if (PlatformDependent.javaVersion() >= 15) {
        try {
            return SelectorProvider.class.getMethod(methodName, java.net.ProtocolFamily.class);
        } catch (Throwable e) {
            logger.debug("SelectorProvider.{}(ProtocolFamily) not available, will use default", methodName, e);
        }
    }
    return null;
}

If Java version higher than or equals to 15, SelectorProviderUtil may invoke java.nio.channels.spi.SelectorProvide by reflection.

SelectorProviderUtil.findOpenMethod will be called in two Netty class: NioSocketChannel and NioServerSocketChannel corresponding to two SelectorProvider methods: openSocketChannel and openServerSocketChannel.

This PR add metadata for them.

Code sections where the PR accesses files, network, docker or some external service

None

Checklist before merging

@czp3009 czp3009 requested a review from a team as a code owner September 6, 2023 08:11
@czp3009 czp3009 requested a review from vjovanov September 6, 2023 08:11
violetagg added a commit to reactor/reactor-netty that referenced this pull request Sep 13, 2023
Temporary add the missing GraalVM metadata to the test as the PRs below are still in progress
oracle/graalvm-reachability-metadata#379
oracle/graalvm-reachability-metadata#382
violetagg added a commit to reactor/reactor-netty that referenced this pull request Sep 13, 2023
Temporary add the missing GraalVM metadata to the test as the PRs below are still in progress
oracle/graalvm-reachability-metadata#379
oracle/graalvm-reachability-metadata#382
@vjovanov vjovanov requested a review from dnestoro September 19, 2023 11:35
@dnestoro
Copy link
Member

dnestoro commented Nov 6, 2023

Can't you use EnabledOnJre annotation (see this) and write unit test which covers described metadata?

@dnestoro
Copy link
Member

@czp3009 is there a plan to work on this PR? Have you tried EnaledOnJre as I mentioned in previous comment?

@nkonev
Copy link
Contributor

nkonev commented Dec 29, 2023

I suppose that the better place is https://github.com/netty/netty - they already have graal-related metadata https://github.com/netty/netty/issues?q=graal

@dnestoro
Copy link
Member

I suppose that the better place is https://github.com/netty/netty - they already have graal-related metadata https://github.com/netty/netty/issues?q=graal

That would be even better!

@czp3009
Copy link
Contributor Author

czp3009 commented Dec 30, 2023

hi @dnestoro , i was also trying to add metadata to Netty repository, but that doesn't solve the problem. please see following issue: netty/netty#13575

After discussing with the Netty repository manager, i was decided to commit the metadata to this repository.

I am very sorry for not submitting test case code for a long time, but I've been really busy lately, and I'm not familiar with test cases like this. I'm not sure when I'll have time to do this PR, at least not in the near future.

@violetagg
Copy link
Contributor

@dnestoro The tests below should validate this change.

diff --git a/tests/src/io.netty/netty-common/4.1.80.Final/src/test/java/netty/NettyTests.java b/tests/src/io.netty/netty-common/4.1.80.Final/src/test/java/netty/NettyTests.java
index 5df7cc2..a38c1ba 100644
--- a/tests/src/io.netty/netty-common/4.1.80.Final/src/test/java/netty/NettyTests.java
+++ b/tests/src/io.netty/netty-common/4.1.80.Final/src/test/java/netty/NettyTests.java
@@ -7,6 +7,9 @@
 package netty;
 
 import java.io.InputStream;
+import java.net.Inet6Address;
+import java.nio.channels.UnsupportedAddressTypeException;
+import java.nio.channels.spi.SelectorProvider;
 import java.time.Duration;
 import java.util.Objects;
 import java.util.concurrent.atomic.AtomicReference;
@@ -56,8 +59,13 @@ import io.netty.handler.ssl.util.InsecureTrustManagerFactory;
 import io.netty.util.CharsetUtil;
 import org.awaitility.Awaitility;
 import org.hamcrest.CoreMatchers;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Assumptions;
 import org.junit.jupiter.api.Test;
 
+import static io.netty.channel.socket.InternetProtocolFamily.IPv4;
+import static io.netty.util.NetUtil.LOCALHOST;
+
 public class NettyTests {
     private static final int PORT = 8080;
 
@@ -71,6 +79,36 @@ public class NettyTests {
         test(false);
     }
 
+    @Test
+    void testNioSocketChannel() {
+        Assumptions.assumeTrue(LOCALHOST instanceof Inet6Address);
+
+        EventLoopGroup group = new NioEventLoopGroup();
+        try {
+            Bootstrap b = new Bootstrap().group(group)
+                    .channelFactory(() -> new NioSocketChannel(SelectorProvider.provider(), IPv4))
+                    .handler(new LoggingHandler());
+            Assertions.assertThrows(UnsupportedAddressTypeException.class, () -> b.bind(LOCALHOST, 7777).sync().channel());
+        } finally {
+            group.shutdownGracefully();
+        }
+    }
+
+    @Test
+    void testNioServerSocketChannel() {
+        Assumptions.assumeTrue(LOCALHOST instanceof Inet6Address);
+
+        EventLoopGroup group = new NioEventLoopGroup();
+        try {
+            Bootstrap b = new Bootstrap().group(group)
+                    .channelFactory(() -> new NioServerSocketChannel(SelectorProvider.provider(), IPv4))
+                    .handler(new LoggingHandler());
+            Assertions.assertThrows(UnsupportedAddressTypeException.class, () -> b.bind(LOCALHOST, 7777).sync().channel());
+        } finally {
+            group.shutdownGracefully();
+        }
+    }
+
     private void test(boolean ssl) throws Exception {
         EventLoopGroup bossGroup = new NioEventLoopGroup(1);
         EventLoopGroup workerGroup = new NioEventLoopGroup();

@violetagg
Copy link
Contributor

cc @bclozel @sdeleuze

@sdeleuze
Copy link
Collaborator

@czp3009 Can you please update your PR with the tests proposed by @violetagg ?

@czp3009
Copy link
Contributor Author

czp3009 commented Jan 21, 2024

@sdeleuze @dnestoro test case updated

@olpaw olpaw self-requested a review January 22, 2024 14:58
@dnestoro dnestoro merged commit bc3d675 into oracle:master Jan 23, 2024
6 checks passed
@violetagg
Copy link
Contributor

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants