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

SimpleChannelPool#notifyConnect() may leak Channels #5553

Closed
rkapsi opened this issue Jul 19, 2016 · 3 comments
Closed

SimpleChannelPool#notifyConnect() may leak Channels #5553

rkapsi opened this issue Jul 19, 2016 · 3 comments
Assignees
Labels
Milestone

Comments

@rkapsi
Copy link
Member

rkapsi commented Jul 19, 2016

Hey,

I just stumbled onto this one. The SimpleChannelPool#notifyConnect() method will leak Channels if the user cancelled the Promise.

private static void notifyConnect(ChannelFuture future, Promise<Channel> promise) {
        if (future.isSuccess()) {
            // This will leak the Channel if the Promise is complete
            promise.setSuccess(future.channel());
        } else {
            promise.setFailure(future.cause());
        }
    }
private static void notifyConnect(ChannelFuture future, Promise<Channel> promise) {
        if (future.isSuccess()) {
            // Possible fix
            Channel channel = future.channel();
            if (!promise.trySuccess(channel)) {
                release(channel);
            }
        } else {
            promise.setFailure(future.cause());
        }
    }
2016-07-19 14:14:48:400 EDT [32647,NIO-ConnectorThread-0,] WARN DefaultPromise - An exception was thrown by io.netty.channel.pool.SimpleChannelPool$2.operationComplete()
java.lang.IllegalStateException: complete already: DefaultPromise@78ba125f(failure: java.util.concurrent.CancellationException)
    at io.netty.util.concurrent.DefaultPromise.setSuccess(DefaultPromise.java:105)
    at io.netty.channel.pool.SimpleChannelPool.notifyConnect(SimpleChannelPool.java:159)
    at io.netty.channel.pool.SimpleChannelPool.access$000(SimpleChannelPool.java:42)
    at io.netty.channel.pool.SimpleChannelPool$2.operationComplete(SimpleChannelPool.java:134)
    at io.netty.channel.pool.SimpleChannelPool$2.operationComplete(SimpleChannelPool.java:131)
    at io.netty.util.concurrent.DefaultPromise.notifyListener0(DefaultPromise.java:514)
    at io.netty.util.concurrent.DefaultPromise.notifyListeners0(DefaultPromise.java:507)
    at io.netty.util.concurrent.DefaultPromise.notifyListenersNow(DefaultPromise.java:486)
    at io.netty.util.concurrent.DefaultPromise.notifyListeners(DefaultPromise.java:427)
    at io.netty.util.concurrent.DefaultPromise.trySuccess(DefaultPromise.java:111)
    at io.netty.channel.DefaultChannelPromise.trySuccess(DefaultChannelPromise.java:82)
    at io.netty.channel.nio.AbstractNioChannel$AbstractNioUnsafe.fulfillConnectPromise(AbstractNioChannel.java:300)
    at io.netty.channel.nio.AbstractNioChannel$AbstractNioUnsafe.finishConnect(AbstractNioChannel.java:335)
    at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:588)
    at io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:512)
    at io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:426)
    at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:398)
    at io.netty.util.concurrent.SingleThreadEventExecutor$5.run(SingleThreadEventExecutor.java:877)
    at java.lang.Thread.run(Thread.java:745)
@normanmaurer
Copy link
Member

@rkapsi good catch! Let me come up with a fix

@normanmaurer normanmaurer self-assigned this Jul 19, 2016
@normanmaurer normanmaurer added this to the 4.0.40.Final milestone Jul 19, 2016
normanmaurer added a commit that referenced this issue Jul 20, 2016
Motivation:

The SimpleChannelPool#notifyConnect() method will leak Channels if the user cancelled the Promise in between.

Modifications:

Release the channel if the Promise was complete before.

Result:

No more channel leaks.
@normanmaurer
Copy link
Member

@rkapsi PTAL #5557

normanmaurer added a commit that referenced this issue Jul 20, 2016
Motivation:

The SimpleChannelPool#notifyConnect() method will leak Channels if the user cancelled the Promise in between.

Modifications:

Release the channel if the Promise was complete before.

Result:

No more channel leaks.
normanmaurer added a commit that referenced this issue Jul 20, 2016
Motivation:

The SimpleChannelPool#notifyConnect() method will leak Channels if the user cancelled the Promise in between.

Modifications:

Release the channel if the Promise was complete before.

Result:

No more channel leaks.
@normanmaurer
Copy link
Member

Fixed by #5557

liuzhengyang pushed a commit to liuzhengyang/netty that referenced this issue Sep 10, 2017
Motivation:

The SimpleChannelPool#notifyConnect() method will leak Channels if the user cancelled the Promise in between.

Modifications:

Release the channel if the Promise was complete before.

Result:

No more channel leaks.
pulllock pushed a commit to pulllock/netty that referenced this issue Oct 19, 2023
Motivation:

The SimpleChannelPool#notifyConnect() method will leak Channels if the user cancelled the Promise in between.

Modifications:

Release the channel if the Promise was complete before.

Result:

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

No branches or pull requests

2 participants