ProxyProtocol: free pp_info heap on NetVConnection recycle#13293
Merged
moonchen merged 1 commit intoJun 18, 2026
Conversation
NetVConnection embeds a ProxyProtocol by value, and a PROXY v2 header parsed by has_proxy_protocol() heap-allocates ProxyProtocol::additional_data (the TLV blob) plus the tlv map. The NetVConnection ClassAllocators are Destruct_on_free=false, so ~ProxyProtocol never runs when a VC is recycled and UnixNetVConnection::clear() did not release pp_info -- the next placement-new on the recycled slot abandoned the buffer and map nodes, leaking once per recycled connection that carried a PROXY v2 header. Behind a PROXY-protocol load balancer that is every inbound connection. Add ProxyProtocol::reset() and call it from UnixNetVConnection::clear() (the single recycle chokepoint for the Unix, SSL and QUIC VCs). reset() swaps the members with empty containers rather than clear()ing them, because clear() retains capacity that the recycle would still abandon. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a per-connection heap leak when UnixNetVConnection instances are recycled after parsing PROXY v2 headers. Because netVCAllocator is configured with Destruct_on_free=false, embedded members’ destructors (including ProxyProtocol) don’t run on recycle, so heap owned by pp_info must be explicitly released during clear().
Changes:
- Add
ProxyProtocol::reset()to release heap-owned state and restore default values. - Call
pp_info.reset()fromUnixNetVConnection::clear()to ensure recycled VCs don’t leak PROXY-protocol state. - Add a unit test covering
ProxyProtocol::reset()and its idempotency.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/iocore/net/UnixNetVConnection.cc |
Resets pp_info during clear() so PROXY v2 heap allocations don’t persist across VC recycle. |
include/iocore/net/ProxyProtocol.h |
Introduces ProxyProtocol::reset() to drop TLVs/additional data and restore default fields. |
src/iocore/net/unit_tests/test_ProxyProtocol.cc |
Adds a unit test ensuring reset() clears parsed PROXY v2 state and is safe to call repeatedly. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
NetVConnectionembedsProxyProtocol pp_infoby value, andProxyProtocolowns heap (theadditional_datastring and the parsedtlvmap).has_proxy_protocol()parses an inbound PROXY v2 header intopp_infoonce per connection, allocating both.The NetVC
ClassAllocators areDestruct_on_free=false, so~ProxyProtocolnever runs when a VC is recycled andUnixNetVConnection::clear()did not releasepp_info. The slot's next placement-newthen abandons the string buffer and map nodes — leaking once per recycled connection that carried a PROXY v2 header, i.e. every inbound connection behind a PROXY-protocol load balancer.Fix
Add
ProxyProtocol::reset()and call it fromUnixNetVConnection::clear(), the single recycle chokepoint (SSL and QUICclear()chain to it, andclear()runs only on the free path).reset()swaps the members with empty containers rather thanclear()ing them, sinceclear()keeps capacity that the recycle would still abandon. Releasingpp_infoinclear()is safe: it is read only while the VC is live.Targeted rather than flipping the allocators to
Destruct_on_free=true, which would run the full destructor chain on top of the existing manualclear()/free_thread()teardown and needs a separate idempotency audit.