Additional safety guards in stream queue consumer iteration#5932
Merged
Additional safety guards in stream queue consumer iteration#5932
Conversation
harrishancock
approved these changes
Jan 20, 2026
This comment was marked as outdated.
This comment was marked as outdated.
Previously, the `allConsumers` set in queue.h was a collection of raw pointers to consumers. During iteration over the collection or even a snapshot of it, if a consumer was removed and destroyed, it could lead to dereferencing a dangling pointer, causing undefined behavior. This PR modifies `allConsumers` (and SmallSet) to hold WeakRefs instead and tightens up the safety checks during iteration to ensure that we only dereference valid consumers.
6f4cbeb to
b9c453f
Compare
Collaborator
Author
|
@harrishancock ... I've reworked the fix here to use WeakRefs for the set of consumers. The use of the raw pointers always bothered me. Since the impl has been updated entirely since your original review, I dismissed it. |
harrishancock
approved these changes
Jan 21, 2026
Co-authored-by: Harris Hancock <harris@cloudflare.com>
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.
Some additional safety checks when iterating over streams queue consumers.
(opencode did the test updates)