Fix a deadlock bug in the rpc client. The panic will trigger

regularly when client connections are flaky (probably another
issue).

(credits to jussi@tinkercad.com for finding the issue)

R=rsc, r
CC=golang-dev, jussi
https://golang.org/cl/2831042

diff --git a/src/pkg/rpc/client.go b/src/pkg/rpc/client.go
index 2f52d19c6ef0f2c2ad17d98bbae03891e7e76f19..601c49715b351b0e95ae6aae604d23b756d8f454 100644 (file)
--- a/src/pkg/rpc/client.go
+++ b/src/pkg/rpc/client.go
@@ -69,12 +69,12 @@ func (client *Client) send(c *Call) {
        // Encode and send the request.
        request := new(Request)
        client.sending.Lock()
+       defer client.sending.Unlock()
        request.Seq = c.seq
        request.ServiceMethod = c.ServiceMethod
        if err := client.codec.WriteRequest(request, c.Args); err != nil {
                panic("rpc: client encode error: " + err.String())
        }
-       client.sending.Unlock()
 }
 
 func (client *Client) input() {

diff --git a/src/pkg/rpc/server_test.go b/src/pkg/rpc/server_test.go
index e826904c2dd56958458315c4b7df5c00c2619492..355d51ce46b828c99729c9d3cc285c6fa70a677f 100644 (file)
--- a/src/pkg/rpc/server_test.go
+++ b/src/pkg/rpc/server_test.go
@@ -13,6 +13,7 @@ import (
        "strings"
        "sync"
        "testing"
+       "time"
 )
 
 var (
@@ -332,3 +333,52 @@ func TestRegistrationError(t *testing.T) {
                t.Errorf("expected error registering ReplyNotPublic")
        }
 }
+
+type WriteFailCodec int
+
+func (WriteFailCodec) WriteRequest(*Request, interface{}) os.Error {
+       // the panic caused by this error used to not unlock a lock.
+       return os.NewError("fail")
+}
+
+func (WriteFailCodec) ReadResponseHeader(*Response) os.Error {
+       time.Sleep(60e9)
+       panic("unreachable")
+}
+
+func (WriteFailCodec) ReadResponseBody(interface{}) os.Error {
+       time.Sleep(60e9)
+       panic("unreachable")
+}
+
+func (WriteFailCodec) Close() os.Error {
+       return nil
+}
+
+func TestSendDeadlock(t *testing.T) {
+       client := NewClientWithCodec(WriteFailCodec(0))
+
+       done := make(chan bool)
+       go func() {
+               testSendDeadlock(client)
+               testSendDeadlock(client)
+               done <- true
+       }()
+       for i := 0; i < 50; i++ {
+               time.Sleep(100 * 1e6)
+               _, ok := <-done
+               if ok {
+                       return
+               }
+       }
+       t.Fatal("deadlock")
+}
+
+func testSendDeadlock(client *Client) {
+       defer func() {
+               recover()
+       }()
+       args := &Args{7, 8}
+       reply := new(Reply)
+       client.Call("Arith.Add", args, reply)
+}