New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix all reports from go test -race
in weavedns unit tests
#935
Conversation
linter is unhappy. |
@inercia aside from the linter issue, are you happy with the rest? |
select { | ||
case action := <-actionChan: | ||
if action == nil { | ||
c.listener.Shutdown() | ||
c.running = false | ||
return |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
@rade I've added a couple of comments.... |
…avoid race-condition
The shuffleAnswers() call ends up acting on the same data, so it seems simplest to resolve ownership arguments by copying.
This avoids race-conditions in unit tests where the initialisation is on a different goroutine to the logging.
Specifically, move the part that blocks, and call that as necessary. This means Start() can run on the caller's goroutine and avoid race conditions.
I re-did the last commit with an |
Thanks, @bboreham, it looks great now! Everything looks good to me now. Please merge it if all tests pass... |
All unit and smoke tests passed.
Most are artefacts of the test code, but having so many reports made it difficult to check for any real problems.
The only one which looks like a real problem is a34d2ec, though aa85e19 could conceivably result in a crash.