code review 2831042: Fix a deadlock bug in the rpc client. The panic will trigger

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)

[kaib, 2010-11-02 19:09:47]

Hello rsc (cc: golang-dev@googlegroups.com, jussi@tinkercad.com),

I'd like you to review this change.

[rsc, 2010-11-02 19:12:25]

how could these two possibly be different?

russ

[rsc, 2010-11-02 19:13:45]

LGTM

oh i see.  you are guarding against the panic.
ok.

[rsc, 2010-11-02 19:13:57]

but please add a test

[kaib, 2010-11-02 20:32:47]

ptal

[r, 2010-11-02 20:36:50]

http://codereview.appspot.com/2831042/diff/17001/src/pkg/rpc/server_test.go
File src/pkg/rpc/server_test.go (right):

http://codereview.appspot.com/2831042/diff/17001/src/pkg/rpc/server_test.go#n...
src/pkg/rpc/server_test.go:353: func (_ WriteFailCodec) Close() os.Error {
you don't need these underscores.

[kaib, 2010-11-02 20:52:52]

ptal

http://codereview.appspot.com/2831042/diff/17001/src/pkg/rpc/server_test.go
File src/pkg/rpc/server_test.go (right):

http://codereview.appspot.com/2831042/diff/17001/src/pkg/rpc/server_test.go#n...
src/pkg/rpc/server_test.go:353: func (_ WriteFailCodec) Close() os.Error {
On 2010/11/02 20:36:50, r wrote:
> you don't need these underscores.

Done.

[rsc1, 2010-11-02 21:00:01]

LGTM but wait for r

http://codereview.appspot.com/2831042/diff/23001/src/pkg/rpc/server_test.go
File src/pkg/rpc/server_test.go (right):

http://codereview.appspot.com/2831042/diff/23001/src/pkg/rpc/server_test.go#n...
src/pkg/rpc/server_test.go:340: return os.NewError("fail")
// the panic caused by this error used to not unlock a lock.

http://codereview.appspot.com/2831042/diff/23001/src/pkg/rpc/server_test.go#n...
src/pkg/rpc/server_test.go:366: for i := 0; i < 50; i += 1 {
i++

[kaib, 2010-11-02 21:04:06]

ptal (again..)

http://codereview.appspot.com/2831042/diff/23001/src/pkg/rpc/server_test.go
File src/pkg/rpc/server_test.go (right):

http://codereview.appspot.com/2831042/diff/23001/src/pkg/rpc/server_test.go#n...
src/pkg/rpc/server_test.go:340: return os.NewError("fail")
On 2010/11/02 21:00:01, rsc1 wrote:
> // the panic caused by this error used to not unlock a lock.

Done.

http://codereview.appspot.com/2831042/diff/23001/src/pkg/rpc/server_test.go#n...
src/pkg/rpc/server_test.go:366: for i := 0; i < 50; i += 1 {
On 2010/11/02 21:00:01, rsc1 wrote:
> i++

Done.

[r, 2010-11-02 21:04:15]

LGTM
i will submit

[r, 2010-11-02 21:05:01]

*** Submitted as http://code.google.com/p/go/source/detail?r=c9d80617e41f ***

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
http://codereview.appspot.com/2831042

Committer: Rob Pike <r@golang.org>