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

Issue with QuicCodecDispatcher #710

Closed
poisonriver opened this issue Apr 17, 2024 · 13 comments
Closed

Issue with QuicCodecDispatcher #710

poisonriver opened this issue Apr 17, 2024 · 13 comments
Milestone

Comments

@poisonriver
Copy link
Contributor

poisonriver commented Apr 17, 2024

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:

  • Using Curl: 1 packet come from client during Quic connection establishment (Initial) and 1 QuichChannel init call happens
  • Using Chrome : 5 packets come from a client during Quic connection establishment (Initial, 0-RTT, 0-RTT, Initial, Initial) and 5 QuichChannel init calls happen, while Http3 request is being processed only in one of these QuicChannels.

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
Copy link
Member

Can you also take a heap dump and share it ?

@poisonriver
Copy link
Contributor Author

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

@poisonriver
Copy link
Contributor Author

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

@normanmaurer
Copy link
Member

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

@normanmaurer
Copy link
Member

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

@normanmaurer
Copy link
Member

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

@normanmaurer normanmaurer added this to the 0.0.64.Final milestone Jun 7, 2024
@normanmaurer
Copy link
Member

Fixed by #720 and #721

@poisonriver
Copy link
Contributor Author

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
Copy link
Member

@poisonriver keep me posted

@normanmaurer
Copy link
Member

@poisonriver any updates ?

@poisonriver
Copy link
Contributor Author

@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
Copy link
Member

normanmaurer commented Jun 18, 2024

@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
Copy link
Contributor Author

@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

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

No branches or pull requests

2 participants