search

braintree / manners

A polite Go HTTP server that shuts down gracefully.
MIT License
997 stars 103 forks source link

race condition in tests #15

Closed kornel661 closed 9 years ago

kornel661 commented 10 years ago

It seems to me that line 40

<-done

should be moved to the end of the function to avoid a race condition between:

I'm aware that this test runs only on one thread and the handler takes quite long to complete (so the race is highly unlikely), but I believe that the Go memory model makes this change mandatory (to get correct, predictable results according to the language semantics).

Another race is in the TestShutdown. Concurrent execution may lead to a situation where the server is still running but the other goroutine went ahead and successfully connected.

lionelbarrow commented 10 years ago

Given the change in your pull request, I think we can eliminate the sleeps in the test helper. Thanks!

lionelbarrow commented 10 years ago

Turns out we can't -- the change in the PR breaks the test. I'll leave this issue open until we've fixed this.

lionelbarrow commented 9 years ago

This has been fixed as part of the rewrite.