Skip to content

Additional safety guards in stream queue consumer iteration#5932

Merged
jasnell merged 2 commits intomainfrom
jasnell/streams-more-safety-checks
Jan 21, 2026
Merged

Additional safety guards in stream queue consumer iteration#5932
jasnell merged 2 commits intomainfrom
jasnell/streams-more-safety-checks

Conversation

@jasnell
Copy link
Copy Markdown
Collaborator

@jasnell jasnell commented Jan 20, 2026

Some additional safety checks when iterating over streams queue consumers.

(opencode did the test updates)

@jasnell jasnell requested review from a team as code owners January 20, 2026 15:21
@codspeed-hq

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.
@jasnell jasnell force-pushed the jasnell/streams-more-safety-checks branch from 6f4cbeb to b9c453f Compare January 20, 2026 17:45
@jasnell jasnell requested a review from harrishancock January 20, 2026 17:45
@jasnell
Copy link
Copy Markdown
Collaborator Author

jasnell commented Jan 20, 2026

@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.

Co-authored-by: Harris Hancock <harris@cloudflare.com>
@jasnell jasnell enabled auto-merge (squash) January 21, 2026 14:16
@jasnell jasnell merged commit d19e973 into main Jan 21, 2026
34 of 36 checks passed
@jasnell jasnell deleted the jasnell/streams-more-safety-checks branch January 21, 2026 15: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