Fix races in WaitForState

[jbardin]

The WaitForState method can't read the result values in a timeout
because they are still owned by the running goroutine. Keep all values
scoped inside the goroutine, and save them into an atomic.Value to be
returned.

Fixes race introduced in #8510

[stack72]

@radeksimko FYI - i missed this :(

[radeksimko]

@stack72 Thanks for the ping.
@jbardin Thanks for discovering & fixing this.

Not that it's an excuse, but I believe the bug was there before #8510 , affecting resulterr. #8510 was just adding a new variable that became affected by that bug too. 😞

I can see now that the golang race detector would have discovered this. Maybe we could run the whole test suite once a day with the -race flag? Running it for every single time would be CPU/mem/time consuming.

[jbardin]

@radeksimko Oh yes, your PR wasn't the primary culprit. The existing race was just missed because the errors paths weren't sufficiently exercised in the tests.

I'm testing for races thoroughly myself right now, and we have a separate integration test that runs make testrace to flag issues. Once the concurrency issues in core are sorted out I intend to make -race the default.

[mitchellh]

LGTM

[ghost]

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.