chore: refactor to use modern atomic types#3277
Merged
Conversation
Sahil-4555
force-pushed
the
refactor-atomic-types
branch
from
198bd93 to
a36a9b8
Compare
puellanivis
reviewed
Sep 2, 2025
Collaborator
puellanivis
left a comment
puellanivis
left a comment
There was a problem hiding this comment.
Just a few suggestions.
Signed-off-by: Sahil-4555 <[email protected]>
Signed-off-by: Sahil-4555 <[email protected]>
Sahil-4555
force-pushed
the
refactor-atomic-types
branch
from
659dc7a to
32abe92
Compare
Signed-off-by: Sahil-4555 <[email protected]>
Sahil-4555
force-pushed
the
refactor-atomic-types
branch
from
aaab49a to
2fb0acd
Compare
dnwe
changed the title
puellanivis
reviewed
Sep 2, 2025
Collaborator
puellanivis
left a comment
puellanivis
left a comment
There was a problem hiding this comment.
I don’t see anything to comment on. 👍
dnwe
approved these changes
Sep 2, 2025
Collaborator
dnwe
left a comment
dnwe
left a comment
There was a problem hiding this comment.
Thanks! A good piece of housekeeping
dnwe
pushed a commit
that referenced
this pull request
Dec 17, 2025
This completes the change in #3277 and rolls out all atomics usage to using the types, rather than the bare functions. This already found a few usages that had inconsistently used atomic functions on some values. --------- Signed-off-by: Cassondra Foesch <[email protected]>
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
3 participants
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.
What type of PR is this?
What does this PR do? Why is it needed?
This PR updates our atomic operations to use
Go 1.19'snewer typed atomic approach instead of the old function-based method. We're changing fields likeint64toatomic.Int64so we can write cleaner code likechild.retries.Add(1)instead ofatomic.AddInt32(&child.retries, 1). This removes the need for pointer handling and gives us better compile-time safety when multiple threads access the same variables in operations.Which issues(s) does this PR fix?
The function-based atomics allowed accidental non-atomic access to shared variables, causing race conditions in concurrent operations. The verbose
atomic.AddInt32(&child.retries, 1)syntax was harder to maintain. Typed atomics enforce atomic-only access at compile time and use cleaner method calls, preventing threading bugs.