Use atomic variables for ConnectionInterrupted and LBQ currentSize #100

Open

andygrundman wants to merge 1 commit from andygrundman/andyg.atomic-fixes into master

andygrundman commented

2024-12-27 14:23:13 -06:00

(Migrated from github.com)

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.

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.

cgutman commented

2025-01-05 16:54:37 -06:00

(Migrated from github.com)

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 :(

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 :(

This pull request is broken due to missing fork information.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin 'andygrundman/andyg.atomic-fixes:andygrundman/andyg.atomic-fixes'

git switch 'andygrundman/andyg.atomic-fixes'

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch 'master'

git merge --no-ff 'andygrundman/andyg.atomic-fixes'

git switch 'andygrundman/andyg.atomic-fixes'

git rebase 'master'

git switch 'master'

git merge --ff-only 'andygrundman/andyg.atomic-fixes'

git switch 'andygrundman/andyg.atomic-fixes'

git rebase 'master'

git switch 'master'

git merge --no-ff 'andygrundman/andyg.atomic-fixes'

git switch 'master'

git merge --squash 'andygrundman/andyg.atomic-fixes'