dubbo icon indicating copy to clipboard operation
dubbo copied to clipboard

[Bug] Channel Leak in NettyConnectionClient and NettyPortUnificationServer

Open PaulTan94 opened this issue 10 months ago • 0 comments

Pre-check

  • [X] I am sure that all the content I provide is in English.

Search before asking

  • [X] I had searched in the issues and found no similar issues.

Apache Dubbo Component

Java SDK (apache/dubbo)

Dubbo Version

Dubbo java 3.2

Steps to reproduce this issue

  1. Improper handling of new channel in NettyConnectionClient#onConnected and NettyConnectionClient#onGoaway which could leak to a channel leak in Triple client and server.
    @Override
    public void onConnected(Object channel) {
        if (!(channel instanceof io.netty.channel.Channel)) {
            return;
        }
        io.netty.channel.Channel nettyChannel = ((io.netty.channel.Channel) channel);
        if (isClosed()) {
            nettyChannel.close();
            if (LOGGER.isDebugEnabled()) {
                LOGGER.debug(String.format("%s is closed, ignoring connected event", this));
            }
            return;
        }
        this.channel.set(nettyChannel);   // should close the existing channel before setting new channel
        // This indicates that the connection is available.
        if (this.connectingPromise.get() != null) {
            this.connectingPromise.get().trySuccess(CONNECTED_OBJECT);
        }
        nettyChannel.attr(CONNECTION).set(this);
        if (LOGGER.isDebugEnabled()) {
            LOGGER.debug(String.format("%s connected ", this));
        }
    }
    @Override
    public void onGoaway(Object channel) {
        if (!(channel instanceof io.netty.channel.Channel)) {
            return;
        }
        io.netty.channel.Channel nettyChannel = (io.netty.channel.Channel) channel;
        if (this.channel.compareAndSet(nettyChannel, null)) { // should close the existing channel before setting null
            NettyChannel.removeChannelIfDisconnected(nettyChannel);
            if (LOGGER.isDebugEnabled()) {
                LOGGER.debug(String.format("%s goaway", this));
            }
        }
    }

What you expected to happen

  1. Close the existing channel before setting new channel

Anything else

No response

Are you willing to submit a pull request to fix on your own?

  • [X] Yes I am willing to submit a pull request on my own!

Code of Conduct

PaulTan94 avatar Apr 17 '24 12:04 PaulTan94