From a270ea285454768230e517c2853f3cdc8cce8697 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Prante?= Date: Fri, 15 Mar 2024 13:47:36 +0100 Subject: [PATCH] do not use ServiceLoader instances across threads --- gradle.properties | 2 +- .../client/netty/secure/test/Https1Test.java | 3 +- .../http/client/netty/NettyHttpClient.java | 19 ++-- .../client/netty/NettyHttpClientBuilder.java | 81 ++++++++-------- .../http/server/netty/NettyHttpServer.java | 36 ++++---- .../server/netty/NettyHttpServerBuilder.java | 92 +++++++++---------- 6 files changed, 116 insertions(+), 117 deletions(-) diff --git a/gradle.properties b/gradle.properties index 3a97768..ad080a0 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,3 +1,3 @@ group = org.xbib name = net-http -version = 4.3.0 +version = 4.4.0 diff --git a/net-http-client-netty-secure/src/test/java/org/xbib/net/http/client/netty/secure/test/Https1Test.java b/net-http-client-netty-secure/src/test/java/org/xbib/net/http/client/netty/secure/test/Https1Test.java index 6a09f4c..e695fc7 100644 --- a/net-http-client-netty-secure/src/test/java/org/xbib/net/http/client/netty/secure/test/Https1Test.java +++ b/net-http-client-netty-secure/src/test/java/org/xbib/net/http/client/netty/secure/test/Https1Test.java @@ -53,8 +53,7 @@ class Https1Test { @Test void testGoogleHttp() throws Exception { - NettyHttpClientConfig config = new NettyHttpsClientConfig() - .setProtocolNegotiation(true); + NettyHttpClientConfig config = new NettyHttpsClientConfig(); try (NettyHttpClient client = NettyHttpClient.builder() .setConfig(config) .build()) { diff --git a/net-http-client-netty/src/main/java/org/xbib/net/http/client/netty/NettyHttpClient.java b/net-http-client-netty/src/main/java/org/xbib/net/http/client/netty/NettyHttpClient.java index ef9132f..27fb031 100644 --- a/net-http-client-netty/src/main/java/org/xbib/net/http/client/netty/NettyHttpClient.java +++ b/net-http-client-netty/src/main/java/org/xbib/net/http/client/netty/NettyHttpClient.java @@ -37,9 +37,7 @@ public class NettyHttpClient implements HttpClient, C private final AtomicBoolean closed; - private final HttpChannelInitializer httpChannelInitializer; - - private final ServiceLoader httpChannelInitializerServiceLoader; + private HttpChannelInitializer httpChannelInitializer; private Pool pool; @@ -53,7 +51,6 @@ public class NettyHttpClient implements HttpClient, C this.bootstrap = bootstrap; this.closed = new AtomicBoolean(false); this.httpChannelInitializer = builder.httpChannelInitializer; - this.httpChannelInitializerServiceLoader = ServiceLoader.load(HttpChannelInitializer.class); createBoundedPool(builder.nettyHttpClientConfig, bootstrap); this.interactions = new CopyOnWriteArrayList<>(); } @@ -209,13 +206,21 @@ public class NettyHttpClient implements HttpClient, C } + /** + * The lookup here needs to be thread-safe. + * @param httpAddress the HTTP address for the channel initializer to look up. + * @return the channel initializer + */ private HttpChannelInitializer lookupChannelInitializer(HttpAddress httpAddress) { if (httpChannelInitializer != null || httpAddress == null) { return httpChannelInitializer; } - for (HttpChannelInitializer initializer : httpChannelInitializerServiceLoader) { - if (initializer.supports(httpAddress)) { - return initializer; + synchronized (this) { + for (HttpChannelInitializer initializer : ServiceLoader.load(HttpChannelInitializer.class)) { + if (initializer.supports(httpAddress)) { + httpChannelInitializer = initializer; + return initializer; + } } } throw new IllegalStateException("no channel initializer found for address " + httpAddress + ", check service provider"); diff --git a/net-http-client-netty/src/main/java/org/xbib/net/http/client/netty/NettyHttpClientBuilder.java b/net-http-client-netty/src/main/java/org/xbib/net/http/client/netty/NettyHttpClientBuilder.java index 6c361bb..8a57563 100644 --- a/net-http-client-netty/src/main/java/org/xbib/net/http/client/netty/NettyHttpClientBuilder.java +++ b/net-http-client-netty/src/main/java/org/xbib/net/http/client/netty/NettyHttpClientBuilder.java @@ -24,6 +24,8 @@ public class NettyHttpClientBuilder { private static final Logger logger = Logger.getLogger(NettyHttpClientBuilder.class.getName()); + private static final Object lock = new Object(); + NettyHttpClientConfig nettyHttpClientConfig; ByteBufAllocator byteBufAllocator; @@ -93,68 +95,65 @@ public class NettyHttpClientBuilder { if (byteBufAllocator == null) { byteBufAllocator = ByteBufAllocator.DEFAULT; } - EventLoopGroup myEventLoopGroup = createEventLoopGroup(nettyHttpClientConfig, eventLoopGroup); - Class mySocketChannelClass = createChannelClass(nettyHttpClientConfig, socketChannelClass); - Bootstrap bootstrap = createBootstrap(nettyHttpClientConfig, byteBufAllocator, myEventLoopGroup, mySocketChannelClass); + createEventLoopGroup(nettyHttpClientConfig); + createChannelClass(nettyHttpClientConfig); + Bootstrap bootstrap = createBootstrap(nettyHttpClientConfig, byteBufAllocator, eventLoopGroup, socketChannelClass); if (nettyCustomizer != null) { nettyCustomizer.afterBootstrapInitialized(bootstrap); } - return new NettyHttpClient(this, myEventLoopGroup, bootstrap); + return new NettyHttpClient(this, eventLoopGroup, bootstrap); } protected NettyHttpClientConfig createEmptyConfig() { return new NettyHttpClientConfig(); } - private static EventLoopGroup createEventLoopGroup(NettyHttpClientConfig clientConfig, - EventLoopGroup eventLoopGroup) { + private void createEventLoopGroup(NettyHttpClientConfig clientConfig) { if (eventLoopGroup != null) { - return eventLoopGroup; + return; } - EventLoopGroup myEventLoopGroup = null; - ThreadFactory threadFactory = new NamedThreadFactory("org-xbib-net-http-netty-client"); - ServiceLoader transportProviders = ServiceLoader.load(ClientTransportProvider.class); - for (ClientTransportProvider serverTransportProvider : transportProviders) { + synchronized (lock) { + ThreadFactory threadFactory = new NamedThreadFactory("org-xbib-net-http-netty-client"); + for (ClientTransportProvider serverTransportProvider : ServiceLoader.load(ClientTransportProvider.class)) { + if (logger.isLoggable(Level.FINEST)) { + logger.log(Level.FINEST, "found event loop group provider = " + serverTransportProvider); + } + if (clientConfig.getTransportProviderName() == null || clientConfig.getTransportProviderName().equals(serverTransportProvider.getClass().getName())) { + eventLoopGroup = serverTransportProvider.createEventLoopGroup(clientConfig.getThreadCount(), threadFactory); + break; + } + } + if (eventLoopGroup == null) { + eventLoopGroup = new NioEventLoopGroup(clientConfig.getThreadCount(), threadFactory); + } if (logger.isLoggable(Level.FINEST)) { - logger.log(Level.FINEST, "found event loop group provider = " + serverTransportProvider); - } - if (clientConfig.getTransportProviderName() == null || clientConfig.getTransportProviderName().equals(serverTransportProvider.getClass().getName())) { - myEventLoopGroup = serverTransportProvider.createEventLoopGroup(clientConfig.getThreadCount(), threadFactory); - break; + logger.log(Level.FINEST, "event loop group class: " + eventLoopGroup.getClass().getName()); } } - if (myEventLoopGroup == null) { - myEventLoopGroup = new NioEventLoopGroup(clientConfig.getThreadCount(), threadFactory); - } - if (logger.isLoggable(Level.FINEST)) { - logger.log(Level.FINEST, "event loop group class: " + myEventLoopGroup.getClass().getName()); - } - return myEventLoopGroup; } - private static Class createChannelClass(NettyHttpClientConfig clientConfig, - Class socketChannelClass) { + private void createChannelClass(NettyHttpClientConfig clientConfig) { if (socketChannelClass != null) { - return socketChannelClass; + return; } - Class myChannelClass = null; - ServiceLoader transportProviders = ServiceLoader.load(ClientTransportProvider.class); - for (ClientTransportProvider transportProvider : transportProviders) { + synchronized (lock) { + ServiceLoader transportProviders = ServiceLoader.load(ClientTransportProvider.class); + for (ClientTransportProvider transportProvider : transportProviders) { + if (logger.isLoggable(Level.FINEST)) { + logger.log(Level.FINEST, "found socket channel provider = " + transportProvider); + } + if (clientConfig.getTransportProviderName() == null || clientConfig.getTransportProviderName().equals(transportProvider.getClass().getName())) { + socketChannelClass = transportProvider.createSocketChannelClass(); + break; + } + } + if (socketChannelClass == null) { + socketChannelClass = NioSocketChannel.class; + } if (logger.isLoggable(Level.FINEST)) { - logger.log(Level.FINEST, "found socket channel provider = " + transportProvider); - } - if (clientConfig.getTransportProviderName() == null || clientConfig.getTransportProviderName().equals(transportProvider.getClass().getName())) { - myChannelClass = transportProvider.createSocketChannelClass(); - break; + logger.log(Level.FINEST, "socket channel class: " + socketChannelClass.getName()); } } - if (myChannelClass == null) { - myChannelClass = NioSocketChannel.class; - } - if (logger.isLoggable(Level.FINEST)) { - logger.log(Level.FINEST, "socket channel class: " + myChannelClass.getName()); - } - return myChannelClass; } private static Bootstrap createBootstrap(NettyHttpClientConfig nettyHttpClientConfig, diff --git a/net-http-server-netty/src/main/java/org/xbib/net/http/server/netty/NettyHttpServer.java b/net-http-server-netty/src/main/java/org/xbib/net/http/server/netty/NettyHttpServer.java index 92b0804..dbacb32 100644 --- a/net-http-server-netty/src/main/java/org/xbib/net/http/server/netty/NettyHttpServer.java +++ b/net-http-server-netty/src/main/java/org/xbib/net/http/server/netty/NettyHttpServer.java @@ -49,24 +49,18 @@ public class NettyHttpServer implements HttpServer { private final Class socketChannelClass; - private final HttpChannelInitializer httpChannelInitializer; - - private final ServiceLoader serviceLoader; - private final Collection channelFutures; private final Collection channels; - NettyHttpServer(NettyHttpServerBuilder builder, - EventLoopGroup parentEventLoopGroup, - EventLoopGroup childEventLoopGroup, - Class socketChannelClass) { + private HttpChannelInitializer httpChannelInitializer; + + NettyHttpServer(NettyHttpServerBuilder builder) { this.builder = builder; - this.parentEventLoopGroup = parentEventLoopGroup; - this.childEventLoopGroup = childEventLoopGroup; - this.socketChannelClass = socketChannelClass; + this.parentEventLoopGroup = builder.parentEventLoopGroup; + this.childEventLoopGroup = builder.childEventLoopGroup; + this.socketChannelClass = builder.socketChannelClass; this.httpChannelInitializer = builder.httpChannelInitializer; - this.serviceLoader = ServiceLoader.load(HttpChannelInitializer.class); this.channelFutures = new ArrayList<>(); this.channels = new ArrayList<>(); logger.log(Level.FINE, "parent event loop group = " + parentEventLoopGroup + @@ -129,7 +123,8 @@ public class NettyHttpServer implements HttpServer { } }); channel.attr(NettyHttpServerConfig.ATTRIBUTE_HTTP_ADDRESS).set(httpAddress); - createChannelInitializer(httpAddress).init(channel, getServer(), builder.nettyCustomizer); + createChannelInitializer(httpAddress); + httpChannelInitializer.init(channel, getServer(), builder.nettyCustomizer); } }); if (getNettyHttpServerConfig().isDebug()) { @@ -248,15 +243,18 @@ public class NettyHttpServer implements HttpServer { logger.log(Level.INFO, "server shutdown complete"); } - private HttpChannelInitializer createChannelInitializer(HttpAddress address) { + private void createChannelInitializer(HttpAddress address) { if (httpChannelInitializer != null && httpChannelInitializer.supports(address)) { - return httpChannelInitializer; + return; } - for (HttpChannelInitializer httpChannelInitializer : serviceLoader) { - if (httpChannelInitializer.supports(address)) { - return httpChannelInitializer; + synchronized (this) { + for (HttpChannelInitializer httpChannelInitializer : ServiceLoader.load(HttpChannelInitializer.class)) { + if (httpChannelInitializer.supports(address)) { + this.httpChannelInitializer = httpChannelInitializer; + return; + } } + throw new IllegalStateException("no channel initializer found for address " + address); } - throw new IllegalStateException("no channel initializer found for address " + address); } } diff --git a/net-http-server-netty/src/main/java/org/xbib/net/http/server/netty/NettyHttpServerBuilder.java b/net-http-server-netty/src/main/java/org/xbib/net/http/server/netty/NettyHttpServerBuilder.java index b5f1cd5..8b25b9f 100644 --- a/net-http-server-netty/src/main/java/org/xbib/net/http/server/netty/NettyHttpServerBuilder.java +++ b/net-http-server-netty/src/main/java/org/xbib/net/http/server/netty/NettyHttpServerBuilder.java @@ -14,6 +14,8 @@ import java.util.concurrent.ThreadFactory; public class NettyHttpServerBuilder implements HttpServerBuilder { + private static final Object lock = new Object(); + NettyHttpServerConfig nettyHttpServerConfig; Application application; @@ -83,70 +85,66 @@ public class NettyHttpServerBuilder implements HttpServerBuilder { } public NettyHttpServer build() { - return new NettyHttpServer(this, - createParentEventLoopGroup(nettyHttpServerConfig, parentEventLoopGroup), - createChildEventLoopGroup(nettyHttpServerConfig, childEventLoopGroup), - createSocketChannelClass(nettyHttpServerConfig, socketChannelClass)); + createParentEventLoopGroup(nettyHttpServerConfig); + createChildEventLoopGroup(nettyHttpServerConfig); + createSocketChannelClass(nettyHttpServerConfig); + return new NettyHttpServer(this); } - private static EventLoopGroup createParentEventLoopGroup(NettyHttpServerConfig httpServerConfig, - EventLoopGroup parentEventLoopGroup) { + private void createParentEventLoopGroup(NettyHttpServerConfig httpServerConfig) { if (parentEventLoopGroup != null) { - return parentEventLoopGroup; + return; } - EventLoopGroup eventLoopGroup = null; - ThreadFactory threadFactory = new NamedThreadFactory("org-xbib-net-http-netty-server-parent"); - ServiceLoader transportProviders = ServiceLoader.load(ServerTransportProvider.class); - for (ServerTransportProvider serverTransportProvider : transportProviders) { - if (httpServerConfig.getTransportProviderName() == null || - httpServerConfig.getTransportProviderName().equals(serverTransportProvider.getClass().getName())) { - eventLoopGroup = serverTransportProvider.createEventLoopGroup(httpServerConfig.getParentThreadCount(), - threadFactory); + synchronized (lock) { + ThreadFactory threadFactory = new NamedThreadFactory("org-xbib-net-http-netty-server-parent"); + for (ServerTransportProvider serverTransportProvider : ServiceLoader.load(ServerTransportProvider.class)) { + if (httpServerConfig.getTransportProviderName() == null || + httpServerConfig.getTransportProviderName().equals(serverTransportProvider.getClass().getName())) { + parentEventLoopGroup = serverTransportProvider.createEventLoopGroup(httpServerConfig.getParentThreadCount(), + threadFactory); + } + } + if (parentEventLoopGroup == null) { + parentEventLoopGroup = new NioEventLoopGroup(httpServerConfig.getParentThreadCount(), threadFactory); } } - if (eventLoopGroup == null) { - eventLoopGroup = new NioEventLoopGroup(httpServerConfig.getParentThreadCount(), threadFactory); - } - return eventLoopGroup; } - private static EventLoopGroup createChildEventLoopGroup(NettyHttpServerConfig httpServerConfig, - EventLoopGroup childEventLoopGroup) { + private void createChildEventLoopGroup(NettyHttpServerConfig httpServerConfig) { if (childEventLoopGroup != null) { - return childEventLoopGroup; + return; } - EventLoopGroup eventLoopGroup = null; - ThreadFactory threadFactory = new NamedThreadFactory("org-xbib-net-http-netty-server-child"); - ServiceLoader transportProviders = ServiceLoader.load(ServerTransportProvider.class); - for (ServerTransportProvider serverTransportProvider : transportProviders) { - if (httpServerConfig.getTransportProviderName() == null || - httpServerConfig.getTransportProviderName().equals(serverTransportProvider.getClass().getName())) { - eventLoopGroup = serverTransportProvider.createEventLoopGroup(httpServerConfig.getChildThreadCount(), - threadFactory); + synchronized (this) { + ThreadFactory threadFactory = new NamedThreadFactory("org-xbib-net-http-netty-server-child"); + ServiceLoader transportProviders = ServiceLoader.load(ServerTransportProvider.class); + for (ServerTransportProvider serverTransportProvider : transportProviders) { + if (httpServerConfig.getTransportProviderName() == null || + httpServerConfig.getTransportProviderName().equals(serverTransportProvider.getClass().getName())) { + childEventLoopGroup = serverTransportProvider.createEventLoopGroup(httpServerConfig.getChildThreadCount(), + threadFactory); + } + } + if (childEventLoopGroup == null) { + childEventLoopGroup = new NioEventLoopGroup(httpServerConfig.getChildThreadCount(), threadFactory); } } - if (eventLoopGroup == null) { - eventLoopGroup = new NioEventLoopGroup(httpServerConfig.getChildThreadCount(), threadFactory); - } - return eventLoopGroup; } - private static Class createSocketChannelClass(NettyHttpServerConfig httpServerConfig, - Class socketChannelClass) { + private void createSocketChannelClass(NettyHttpServerConfig httpServerConfig) { if (socketChannelClass != null) { - return socketChannelClass; + return; } - Class channelClass = null; - ServiceLoader transportProviders = ServiceLoader.load(ServerTransportProvider.class); - for (ServerTransportProvider serverTransportProvider : transportProviders) { - if (httpServerConfig.getTransportProviderName() == null || httpServerConfig.getTransportProviderName().equals(serverTransportProvider.getClass().getName())) { - channelClass = serverTransportProvider.createServerSocketChannelClass(); - break; + synchronized (lock) { + ServiceLoader transportProviders = ServiceLoader.load(ServerTransportProvider.class); + for (ServerTransportProvider serverTransportProvider : transportProviders) { + if (httpServerConfig.getTransportProviderName() == null || httpServerConfig.getTransportProviderName().equals(serverTransportProvider.getClass().getName())) { + socketChannelClass = serverTransportProvider.createServerSocketChannelClass(); + break; + } + } + if (socketChannelClass == null) { + socketChannelClass = NioServerSocketChannel.class; } } - if (channelClass == null) { - channelClass = NioServerSocketChannel.class; - } - return channelClass; } }