Skip to content

stats: add keyed loop observer removal to fix use-after-free#325

Merged
afrind merged 3 commits into
mainfrom
pr325
May 23, 2026
Merged

stats: add keyed loop observer removal to fix use-after-free#325
afrind merged 3 commits into
mainfrom
pr325

Conversation

@afrind
Copy link
Copy Markdown
Contributor

@afrind afrind commented May 23, 2026

QuicStatsCollector::Callback registered an EVB loop observer capturing
data_.get() as a raw pointer. If the Callback was destroyed while its
EventBaseStatsCollector was still alive, the observer would continue to
fire and dereference the freed pointer.

Fix by giving addLoopObserver a key (typically the owning object's this)
and adding removeLoopObserver(key), which posts an erase to the EVB thread.
Callback's destructor now calls removeLoopObserver so the subscription is
torn down safely. PicoQuicStatsCollector's observer is similarly keyed
for consistency.


Stack created with Sapling. Best reviewed with ReviewStack.


This change is Reviewable

afrind and others added 3 commits May 23, 2026 11:28
TrackConsumerFilter now provides setDownstream() directly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
QuicStatsCollector::Callback registered an EVB loop observer capturing
data_.get() as a raw pointer.  If the Callback was destroyed while its
EventBaseStatsCollector was still alive, the observer would continue to
fire and dereference the freed pointer.

Fix by giving addLoopObserver a key (typically the owning object's `this`)
and adding removeLoopObserver(key), which posts an erase to the EVB thread.
Callback's destructor now calls removeLoopObserver so the subscription is
torn down safely.  PicoQuicStatsCollector's observer is similarly keyed
for consistency.
Copy link
Copy Markdown
Contributor

@gmarzot gmarzot left a comment

Choose a reason for hiding this comment

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

@gmarzot reviewed 7 files and all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on akash-a-n, michalhosna, mondain, Oxyd, peterchave, suhasHere, and TimEvens).

@afrind afrind merged commit 814b064 into main May 23, 2026
23 of 25 checks passed
@afrind afrind deleted the pr325 branch May 23, 2026 23:05
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.

2 participants