Lock before changing field of backends #843
Merged
Conversation
This file contains 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
The `SetBackend` method updates one of the members of the package-level variable `backends`. Since it's a pointer, it's probably safe on most architectures. But the `Backends` struct already has a `sync.RWMutex` field, and earlier in the file the `GetBackend` function goes through the trouble of explicitly acquiring a read and possibly a write lock. See: https://github.com/stripe/stripe-go/blob/d94ce0ed857fb1571b9b30bd24430bdac3d50c0f/stripe.go#L666 So to be safe, and consistent with the rest of the code, it makes sense to acquire a write lock before updating the `backends` value.
|
@chrsmith Yep, sounds good to me! The build is failing because Gofmt needs to be run on your changes. It's not obvious to me from your diff exactly what the problem is though (probably leading or trailing whitespace?). |
|
@brandur-stripe Sorry about that. I made the change in GitHub's code editor, so it's probably some whitespace issue. Fixed. |
|
Awesome, thanks! |
|
Released as 60.12.2. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Just a minor thing I noticed while trying to update a Stripe client to specify
MaxNetworkRetries, since the default is 0. Apologies if this is somehow intentional.The
SetBackendmethod updates one of the members of the package-level variablebackends. Since it's a pointer, it's probably safe on most architectures. But theBackendsstruct already has async.RWMutexfield, and earlier in the file theGetBackendfunction goes through the trouble of explicitly acquiring a read and possibly a write lock. See:stripe-go/stripe.go
Line 666 in d94ce0e
So to be safe, and consistent with the rest of the code, it makes sense to acquire a write lock before updating the
backendsvalue.