netty / netty-incubator-codec-quic

Apache License 2.0
303 stars 72 forks source link

Issue with QuicCodecDispatcher #710

Closed poisonriver closed 5 months ago

poisonriver commented 7 months ago

Hi @normanmaurer , I'm trying to utilize QuicCodecDispatcher and noticed that a number of QuicheQuicChannel instances are increasing with time on my server (when http clients come and leave) thus creating a leak. I removed QuicCodecDispatcher from the server initialization and the problem disappeared. Then I added logging for QuicChannel init and Http3 request processing (Http3RequestStreamInboundHandler.channelRead) and noticed the following:

My server initialization code is below, Wireshark screenshot are attached. Can you please advise if I'm missing something or there's some other issue?

        Bootstrap bootstrap = new Bootstrap().group(nioEventLoopGroup)
                .channel(EpollDatagramChannel.class)
                .option(EpollChannelOption.SO_REUSEPORT, true)
                .handler(new QuicCodecDispatcher() {
                    @Override
                    protected void initChannel(Channel channel, int localConnectionIdLength, QuicConnectionIdGenerator idGenerator) throws Exception {
                        QuicServerCodecBuilder codecBuilder = Http3.newQuicServerCodecBuilder()
                                .localConnectionIdLength(localConnectionIdLength)
                                .connectionIdAddressGenerator(idGenerator)
                                .sslContext(sslContext)
                                .maxIdleTimeout(10000, TimeUnit.MILLISECONDS)
                                .handler(new ChannelInitializer<QuicChannel>() {
                                    protected void initChannel(QuicChannel ch) {
                                        log.info("QuicChannel init: " + ch.id());
                                        ch.pipeline().addLast(new Http3ServerConnectionHandler(
                                                new ChannelInitializer<QuicStreamChannel>() {
                                                    @Override
                                                    protected void initChannel(QuicStreamChannel ch) {
                                                        ch.pipeline().addLast(new Http3RequestStreamInboundHandler() {
                                                            @Override
                                                            protected void channelRead(ChannelHandlerContext ctx, Http3HeadersFrame frame) throws Exception {
                                                            }

                                                            @Override
                                                            protected void channelRead(ChannelHandlerContext ctx, Http3DataFrame frame) throws Exception {
                                                            }

                                                            @Override
                                                            protected void channelInputClosed(ChannelHandlerContext ctx) throws Exception {
                                                            }

                                                        });
                                                    }
                                                }, null, null, null, true)
                                        );
                                    }
                                });
                        channel.pipeline().addLast(codecBuilder.build());
                    }
                });
        for (int i = 0; i < threadsPerAddress; i++) {
            Channel quicChannel = bootstrap
                    .bind(address, port).sync().channel();
            quicChannels.add(quicChannel);
        }
curl-codec-dispatcher chrome-codec-dispatcher
normanmaurer commented 7 months ago

Can you also take a heap dump and share it ?

poisonriver commented 7 months ago

Hi @normanmaurer , heap dump (made with option 'live' ) attached netty-dump.bin.gz

poisonriver commented 7 months ago

Hi @normanmaurer , Did you have a chance to look through the heap dump?

normanmaurer commented 5 months ago

Sorry these last few weeks been super busy. I will pick it up next week

normanmaurer commented 5 months ago

@poisonriver hmm... in the heap dump I only see 10 QuicheQuicChannel instances.

normanmaurer commented 5 months ago

@poisonriver I am pretty sure this should fix what you did see https://github.com/netty/netty-incubator-codec-quic/pull/720

normanmaurer commented 5 months ago

Fixed by https://github.com/netty/netty-incubator-codec-quic/pull/720 and https://github.com/netty/netty-incubator-codec-quic/pull/721

poisonriver commented 5 months ago

Hi @normanmaurer , Thank you, looks like this is the issue, I will test it soon

@poisonriver I am pretty sure this should fix what you did see #720

normanmaurer commented 5 months ago

@poisonriver keep me posted

normanmaurer commented 5 months ago

@poisonriver any updates ?

poisonriver commented 5 months ago

@poisonriver any updates ?

Hi @normanmaurer ran some tests, looks like nothing leaks now, thank you! side question: is there any reason for not exposing initial congestion window from the Quiche config?

normanmaurer commented 5 months ago

@poisonriver any updates ?

Hi @normanmaurer ran some tests, looks like nothing leaks now, thank you! side question: is there any reason for not exposing initial congestion window from the Quiche config?

Nope... PRs welcome :)

poisonriver commented 5 months ago

@poisonriver any updates ?

Hi @normanmaurer ran some tests, looks like nothing leaks now, thank you! side question: is there any reason for not exposing initial congestion window from the Quiche config?

Nope... PRs welcome :)

PR is created