Skip to content

fix(subscriber): changed atomic add to fetch update #629

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

MilesConn
Copy link

@MilesConn MilesConn commented May 28, 2025

Hi I can file a bug if that's needed but on my project (I can try and make an MVE) I kept running into an out of bounds panic originating from

for cs in &self.ptrs[start..len] {

I assumed this was possible due to multiple calls to fetch_add incrementing and thus putting len temporarily in a bad state that could allow an out of bound access. The other solution I thought of would've been len.min(MAX_CALLSITES) so up to you which you prefer. I should note, I'm not super familiar with atomics so I'm unsure if I did it right.

While I don't have an MVE using this modified code with my project I no longer saw the panic. Let me know if there's anything else I can do and thanks for all your hard work! :)

Previously it was possible for multiple callers to increment len
in such a way that an out of bounds access could happen.
@MilesConn MilesConn requested a review from a team as a code owner May 28, 2025 00:14
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.

1 participant