SimpleChannelPool "POOL_KEY" attribute name is easy to get conflict from user code

[nizarm]

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?

Create a channel attribute in user code with name 'channelPool' and when SimpleChannelPool is instantiated - 'channelPool' is already in use exception is thrown. This issue surfaced when we utilized Azure SDK.

private static final AttributeKey<SimpleChannelPool> POOL_KEY = AttributeKey.newInstance("channelPool");

Netty version:4.1.39

Suggested Fix: Use UUID to avoid user code conflict as shown below

private static final AttributeKey<SimpleChannelPool> POOL_KEY = AttributeKey.newInstance("channelPool_" + UUID.randomUUID().toString());

[normanmaurer]

@nizarm fancy to do a pr ?

I think we may even use something like this:

private final AttributeKey<SimpleChannelPool> poolKey = AttributeKey.newInstance("channelPool." +
            System.identityHashCode(this));

Just in case there are multiple SimpleChannelPool instances.

[nizarm]

@nizarm fancy to do a pr ?

I think we may even use something like this:

private final AttributeKey<SimpleChannelPool> poolKey = AttributeKey.newInstance("channelPool." +
            System.identityHashCode(this));

Just in case there are multiple SimpleChannelPool instances.

Yes - on it !

[normanmaurer]

@nizarm ok cool... let me know if you get to busy and so can not finish it as I want to release the next version of netty mid next week and it would be nice to have this as well.

[zzmao]

HI @nizarm @normanmaurer ,

We are using Netty 4.1.42.Final, but we see the following exception twice in a month:

java.lang.IllegalArgumentException: 'channelPool.781942502' is already in use
        at io.netty.util.ConstantPool.createOrThrow(ConstantPool.java:113) ~[netty-all-4.1.42.Final.jar:4.1.42.Final]
        at io.netty.util.ConstantPool.newInstance(ConstantPool.java:95) ~[netty-all-4.1.42.Final.jar:4.1.42.Final]
        at io.netty.util.AttributeKey.newInstance(AttributeKey.java:55) ~[netty-all-4.1.42.Final.jar:4.1.42.Final]
        at io.netty.channel.pool.SimpleChannelPool.<init>(SimpleChannelPool.java:42) ~[netty-all-4.1.42.Final.jar:4.1.42.Final]
        at io.netty.channel.pool.SimpleChannelPool.<init>(SimpleChannelPool.java:85) ~[netty-all-4.1.42.Final.jar:4.1.42.Final]
        at io.netty.channel.pool.SimpleChannelPool.<init>(SimpleChannelPool.java:70) ~[netty-all-4.1.42.Final.jar:4.1.42.Final]
        at io.netty.channel.pool.SimpleChannelPool.<init>(SimpleChannelPool.java:58) ~[netty-all-4.1.42.Final.jar:4.1.42.Final]
        at com.github.ambry.network.http2.Http2MultiplexedChannelPool.<init>(Http2MultiplexedChannelPool.java:102) ~[ambry-network-0.3.266.jar:?]
        at com.github.ambry.network.http2.Http2ChannelPoolMap.newPool(Http2ChannelPoolMap.java:53) ~[ambry-network-0.3.266.jar:?]
        at com.github.ambry.network.http2.Http2ChannelPoolMap.newPool(Http2ChannelPoolMap.java:32) ~[ambry-network-0.3.266.jar:?]
        at io.netty.channel.pool.AbstractChannelPoolMap.get(AbstractChannelPoolMap.java:40) ~[netty-all-4.1.42.Final.jar:4.1.42.Final]
        at com.github.ambry.network.http2.Http2NetworkClient.sendAndPoll(Http2NetworkClient.java:93) ~[ambry-network-0.3.266.jar:?]
        at com.github.ambry.router.NonBlockingRouter$OperationController.run(NonBlockingRouter.java:1005) [ambry-router-0.3.266.jar:?]
        at java.lang.Thread.run(Thread.java:748) [?:1.8.0_172]

The related code is:

public class SimpleChannelPool implements ChannelPool {
    private final AttributeKey<SimpleChannelPool> poolKey = AttributeKey.newInstance("channelPool." +
        System.identityHashCode(this))

Since our code creates brand new SimpleChannelPool, Is it possible that that a SimpleChannelPool was garbage collected, then a new SimpleChannelPool is created at the same memory space and hit this already in use exception?

[cgtz]

I see that this change is rolled back with #9760 and the AttributeKey is static once again in netty 4.1.44+. I guess the assumption with the static key is that there is exactly one SimpleChannelPool per channel but the member variable version would have allowed multiple pool instances to be attached to one channel (with the downside of the potential memory leak described in #9760)?