Skip to content
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

Use atomic variables for ConnectionInterrupted and LBQ currentSize #100

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

Conversation

andygrundman
Copy link

These were the only two variables reported by clang's -fsanitize=thread. I was testing the iOS app in Xcode and didn't test that many code paths, but these 2 were straightforward to fix.

For as often as currentSize gets touched, it's never caused a problem for me, so I doubt these have ever caused crashes for anyone. Maybe technically, in theory they could have.

…hese were the only two variables reported by clang's -fsanitize=thread
@cgutman
Copy link
Member

cgutman commented Jan 5, 2025

Yeah, I tried a while back to make this code TSan clean. Unfortunately we will run into the fact that C11 atomics aren't well supported on MSVC. Last I saw, they are supported with only an experimental compiler flag (/experimental:c11atomics). I just gave it a try with the latest VS 17.12.3 and it still requires the experimental flag :(

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