helper/resource: Fix data race in resource.Retry #4700
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 implementation was attempting to capture the error using a scoped
variable reference, but the error value is already exposed via the
return value of `WaitForState()`. Using that instead fixes the data
race.
Fixes the following data race:
```
==================
WARNING: DATA RACE
Read by goroutine 74:
github.com/hashicorp/terraform/helper/resource.Retry()
/Users/phinze/go/src/github.com/hashicorp/terraform/helper/resource/wait.go:35 +0x284
github.com/hashicorp/terraform/helper/resource.TestRetry_timeout()
/Users/phinze/go/src/github.com/hashicorp/terraform/helper/resource/wait_test.go:35 +0x60
testing.tRunner()
/private/var/folders/vd/7l9ys5k57l91x63sh28wl_kc0000gn/T/workdir/go/src/testing/testing.go:456 +0xdc
Previous write by goroutine 90:
github.com/hashicorp/terraform/helper/resource.Retry.func1()
/Users/phinze/go/src/github.com/hashicorp/terraform/helper/resource/wait.go:20 +0x87
github.com/hashicorp/terraform/helper/resource.(*StateChangeConf).WaitForState.func1()
/Users/phinze/go/src/github.com/hashicorp/terraform/helper/resource/state.go:83 +0x284
Goroutine 74 (running) created at:
testing.RunTests()
/private/var/folders/vd/7l9ys5k57l91x63sh28wl_kc0000gn/T/workdir/go/src/testing/testing.go:561 +0xaa3
testing.(*M).Run()
/private/var/folders/vd/7l9ys5k57l91x63sh28wl_kc0000gn/T/workdir/go/src/testing/testing.go:494 +0xe4
main.main()
github.com/hashicorp/terraform/helper/resource/_test/_testmain.go:84 +0x20f
Goroutine 90 (running) created at:
github.com/hashicorp/terraform/helper/resource.(*StateChangeConf).WaitForState()
/Users/phinze/go/src/github.com/hashicorp/terraform/helper/resource/state.go:127 +0x283
github.com/hashicorp/terraform/helper/resource.Retry()
/Users/phinze/go/src/github.com/hashicorp/terraform/helper/resource/wait.go:34 +0x276
github.com/hashicorp/terraform/helper/resource.TestRetry_timeout()
/Users/phinze/go/src/github.com/hashicorp/terraform/helper/resource/wait_test.go:35 +0x60
testing.tRunner()
/private/var/folders/vd/7l9ys5k57l91x63sh28wl_kc0000gn/T/workdir/go/src/testing/testing.go:456 +0xdc
==================
```
phinze
force-pushed
the
phinze/fix-testrace-resource
branch
from
05bb5c2
to
d59db52
Compare
phinze
changed the title
|
LGTM |
phinze
added a commit
that referenced
this pull request
Jan 19, 2016
helper/resource: Fix data race in resource.Retry
phinze
added a commit
that referenced
this pull request
Mar 4, 2016
In #4700 while fixing a data race I made an incorrect assumption about the return value of StateChangeConf, and ended up changing the behavior in the timeout case to always return: > timeout while waiting for state to become '[success]' When it used to capture the "most recent error" from the function itself. It's much more useful to see that error bubbling up, so here we revert to pulling it out of the function as we did before, and we protect against the data race with a good old fashioned mutex.
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
ghost
locked and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
(part of a series of PRs getting us to green so we can run
go test -raceduring Travis)The implementation was attempting to capture the error using a scoped
variable reference, but the error value is already exposed via the
return value of
WaitForState(). Using that instead fixes the datarace.
Fixes the following data race: