FAQ
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)


--

---
You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Search Discussions

  • Iant at Feb 15, 2013 at 6:57 pm
    LGTM

    https://codereview.appspot.com/7338045/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedFeb 15, '13 at 2:01p
activeFeb 15, '13 at 6:57p
posts2
users2
websitegolang.org

2 users in discussion

Iant: 1 post Rogpeppe: 1 post

People

Translate

site design / logo © 2022 Grokbase