Code review comment for lp:~jameinel/mgo/bug-1191487-close-race

https://codereview.appspot.com/10392043/diff/1/server.go
File server.go (right):

https://codereview.appspot.com/10392043/diff/1/server.go#newcode109
server.go:109: socket.Close()
On 2013/06/18 13:29:04, rog wrote:
> server.Unlock()
> (probably before calling socket.Close, but worth investigating)

Yeah, definitely.

https://codereview.appspot.com/10392043/diff/1/server.go#newcode110
server.go:110: return nil, false, errServerClosed
If we have a closed error (which is a good idea, thanks), it should also
be exercised before the connection is attempted.

https://codereview.appspot.com/10392043/diff/1/server.go#newcode264
server.go:264: } else if err == errServerClosed {
There are now two different ways to check for server closed in this
loop. Can we have a single one now that we have errServerClosed?

https://codereview.appspot.com/10392043/diff/1/server_test.go
File server_test.go (right):

https://codereview.appspot.com/10392043/diff/1/server_test.go#newcode16
server_test.go:16: // The first call to this will be during newServer,
and
I appreciate this test as a good reproducer for the issue you're fixing,
but I'm afraid it's quite fragile and extremely attached to
implementation details. If there's no good way to test it in a more
reliable way, I'm fine without an additional test (note that you arrived
at the fix from breakage in the test suite itself).

https://codereview.appspot.com/10392043/