[golang-dev] code review 7338045: net/rpc: avoid racy use of closing flag. (issue 7338045)

Reviewers: r,

Message:
Hello r@golang.org (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg/


Description:
net/rpc: avoid racy use of closing flag.

It's accessed without mutex protection
in a different goroutine from the one that
sets it.

Also make sure that Client.Call after Client.Close
will reliably return ErrShutdown, and that clients
see ErrShutdown rather than io.EOF when appropriate.

Suggestions welcome for a way to reliably test
the mutex issue.

Please review this at https://codereview.appspot.com/7338045/

Affected files:
M src/pkg/net/rpc/client.go
M src/pkg/net/rpc/server_test.go


Index: src/pkg/net/rpc/client.go
===================================================================
--- a/src/pkg/net/rpc/client.go
+++ b/src/pkg/net/rpc/client.go
@@ -71,7 +71,7 @@

// Register this call.
client.mutex.Lock()
- if client.shutdown {
+ if client.shutdown || client.closing {
call.Error = ErrShutdown
client.mutex.Unlock()
call.done()
@@ -105,9 +105,6 @@
response = Response{}
err = client.codec.ReadResponseHeader(&response)
if err != nil {
- if err == io.EOF && !client.closing {
- err = io.ErrUnexpectedEOF
- }
break
}
seq := response.Seq
@@ -150,6 +147,13 @@
client.mutex.Lock()
client.shutdown = true
closing := client.closing
+ if err == io.EOF {
+ if closing {
+ err = ErrShutdown
+ } else {
+ err = io.ErrUnexpectedEOF
+ }
+ }
for _, call := range client.pending {
call.Error = err
call.done()
Index: src/pkg/net/rpc/server_test.go
===================================================================
--- a/src/pkg/net/rpc/server_test.go
+++ b/src/pkg/net/rpc/server_test.go
@@ -524,6 +524,23 @@
}
}

+func TestErrorAfterClientClose(t *testing.T) {
+ once.Do(startServer)
+
+ client, err := dialHTTP()
+ if err != nil {
+ t.Fatalf("dialing: %v", err)
+ }
+ err = client.Close()
+ if err != nil {
+ t.Fatal("close error:", err)
+ }
+ err = client.Call("Arith.Add", &Args{7, 9}, new(Reply))
+ if err != ErrShutdown {
+ t.Errorf("Forever: expected ErrShutdown got %v", err)
+ }
+}
+
func benchmarkEndToEnd(dial func() (*Client, error), b *testing.B) {
b.StopTimer()
once.Do(startServer)