LGTM with a few thoughts and suggestions regarding the test.
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() server.Unlock() (probably before calling socket.Close, but worth investigating)
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#newcode19 server_test.go:19: if server != nil { is the dialler guaranteed not to be called concurrently?
https://codereview.appspot.com/10392043/diff/1/server_test.go#newcode22 server_test.go:22: return net.DialTCP("tcp", nil, addr.(*net.TCPAddr)) return net.Dial("tcp", addr.String()) ?
https://codereview.appspot.com/10392043/diff/1/server_test.go#newcode41 server_test.go:41: // It is possible for newServer to ping the remote host, and the s/possible/not possible/ ?
https://codereview.appspot.com/10392043/diff/1/server_test.go#newcode52 server_test.go:52: } i'm not sure this is guaranteed to work unless you have a sleep in here (i think you're relying on logf yielding, which doesn't seem quite right).
https://codereview.appspot.com/10392043/
« Back to merge proposal
LGTM with a few thoughts and suggestions regarding the test.
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()
server.Unlock()
(probably before calling socket.Close, but worth investigating)
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# newcode19
server_test.go:19: if server != nil {
is the dialler guaranteed not to be called concurrently?
https:/ /codereview. appspot. com/10392043/ diff/1/ server_ test.go# newcode22 net.TCPAddr) )
server_test.go:22: return net.DialTCP("tcp", nil, addr.(*
return net.Dial("tcp", addr.String()) ?
https:/ /codereview. appspot. com/10392043/ diff/1/ server_ test.go# newcode41
server_test.go:41: // It is possible for newServer to ping the remote
host, and the
s/possible/not possible/
?
https:/ /codereview. appspot. com/10392043/ diff/1/ server_ test.go# newcode52
server_test.go:52: }
i'm not sure this is guaranteed to work unless you have a sleep in here
(i think you're relying on logf yielding, which doesn't seem quite
right).
https:/ /codereview. appspot. com/10392043/