From 572b6a8f196ad619859ab0a4bd391a7c405d5f2a Mon Sep 17 00:00:00 2001 From: Nizar Mankulangara Date: Mon, 9 Sep 2019 00:47:23 -0700 Subject: [PATCH] SimpleChannelPool POOL_KEY attribute name is easy to get conflict from user code (#9542) (#9548) Motivation: It is noticed that SimpleChannelPool's POOL_KEY attribute name channelPool is easy to get conflict with user code and throws an exception 'channelPool' is already in use. Being a generic framework - it would be great if we can name the attribute something unique - may be use UUID for the name since the name is not required later. Modifications: This change make sure that the POOL_KEY used inside SimpleChannelPool is unique by appending the object hashcode in the name. Result: No unwanted channel attribute name conflict with user code. --- .../io/netty/channel/pool/SimpleChannelPool.java | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/transport/src/main/java/io/netty/channel/pool/SimpleChannelPool.java b/transport/src/main/java/io/netty/channel/pool/SimpleChannelPool.java index fe711fbfb2..74a6d410c4 100644 --- a/transport/src/main/java/io/netty/channel/pool/SimpleChannelPool.java +++ b/transport/src/main/java/io/netty/channel/pool/SimpleChannelPool.java @@ -39,7 +39,8 @@ import static io.netty.util.internal.ObjectUtil.*; * */ public class SimpleChannelPool implements ChannelPool { - private static final AttributeKey POOL_KEY = AttributeKey.newInstance("channelPool"); + private final AttributeKey poolKey = AttributeKey.newInstance("channelPool." + + System.identityHashCode(this)); private final Deque deque = PlatformDependent.newConcurrentDeque(); private final ChannelPoolHandler handler; private final ChannelHealthChecker healthCheck; @@ -171,7 +172,7 @@ public class SimpleChannelPool implements ChannelPool { if (ch == null) { // No Channel left in the pool bootstrap a new Channel Bootstrap bs = bootstrap.clone(); - bs.attr(POOL_KEY, this); + bs.attr(poolKey, this); ChannelFuture f = connectChannel(bs); if (f.isDone()) { notifyConnect(f, promise); @@ -237,7 +238,7 @@ public class SimpleChannelPool implements ChannelPool { if (future.isSuccess()) { if (future.getNow()) { try { - ch.attr(POOL_KEY).set(this); + ch.attr(poolKey).set(this); handler.channelAcquired(ch); promise.setSuccess(ch); } catch (Throwable cause) { @@ -293,7 +294,7 @@ public class SimpleChannelPool implements ChannelPool { private void doReleaseChannel(Channel channel, Promise promise) { assert channel.eventLoop().inEventLoop(); // Remove the POOL_KEY attribute from the Channel and check if it was acquired from this pool, if not fail. - if (channel.attr(POOL_KEY).getAndSet(null) != this) { + if (channel.attr(poolKey).getAndSet(null) != this) { closeAndFail(channel, // Better include a stacktrace here as this is an user error. new IllegalArgumentException( @@ -357,12 +358,12 @@ public class SimpleChannelPool implements ChannelPool { } } - private static void closeChannel(Channel channel) { - channel.attr(POOL_KEY).getAndSet(null); + private void closeChannel(Channel channel) { + channel.attr(poolKey).getAndSet(null); channel.close(); } - private static void closeAndFail(Channel channel, Throwable cause, Promise promise) { + private void closeAndFail(Channel channel, Throwable cause, Promise promise) { closeChannel(channel); promise.tryFailure(cause); }