Skip to content

http: free up quic listener when stopping#7177

Merged
mholt merged 1 commit intomasterfrom
clean-h3-listeners
Aug 13, 2025
Merged

http: free up quic listener when stopping#7177
mholt merged 1 commit intomasterfrom
clean-h3-listeners

Conversation

@WeidiDeng
Copy link
Member

Fix 7172.

@WeidiDeng WeidiDeng requested review from francislavoie and mholt and removed request for mholt August 13, 2025 06:34
@vnxme
Copy link
Contributor

vnxme commented Aug 13, 2025

Hi, does it also help to implement UDP wrappers the way TCP ones (i.e., listener wrappers) are implemented?

@WeidiDeng
Copy link
Member Author

No, this only frees up resources, not adds new interfaces to facilitate udp multiplexing.

Copy link
Member

@mholt mholt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Weidi!

That is definitely weird, though, to have to call Close() twice, once before application-layer (HTTP server) shutdown and once after app-layer shutdown.

Is that necessary due to the nature of H3/UDP? Like how connections are emulated concepts in H3 maybe?

@mholt mholt added this to the v2.10.1 milestone Aug 13, 2025
@mholt mholt merged commit 7590c9c into master Aug 13, 2025
26 checks passed
@mholt mholt deleted the clean-h3-listeners branch August 13, 2025 18:35
@WeidiDeng
Copy link
Member Author

The double Close is weird, but I don't want to introduce a new method.

In the same version that quic-go stopped closing user provided listeners, closed listeners won't affect accepted connections. It used to be that closing a listener will close all the connections as well.

Yes, it's how quic connections work, all the quic connections are user space implementation of a udp socket multiplexer. So quic-go never closes the udp socket even when the quic listener is closed.

Caddy needs to close the udp socket. As documented, the first Close is to stop new connections, the second is to release packet conns.

@mholt
Copy link
Member

mholt commented Aug 13, 2025

Gotcha, makes sense. I think we can give this a try!

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

Successfully merging this pull request may close these issues.

http: http3 will leak quic listener resources

3 participants