Fix deadlock when using Client + Service over io.Pipe

Client + Server might deadlock while being connected via io.Pipe with
high request concurrency. The scenario is as follows:

  - Server is trying to send reply to a previous client's packet and is
    waiting on Pipe's condvar for the client to read it:

 3  0x0000000000460e39 in sync.(*Cond).Wait
    at /usr/lib/go/src/sync/cond.go:57
 4  0x0000000000464519 in io.(*pipe).write
    at /usr/lib/go/src/io/pipe.go:90
 5  0x0000000000464a4c in io.(*PipeWriter).Write
    at /usr/lib/go/src/io/pipe.go:157
 6  0x00000000007a0b66 in go.(*struct { io.Reader; io.WriteCloser }).Write
    at <autogenerated>:246
 7  0x0000000000790cf6 in github.com/pkg/sftp.(*conn).Write
    at <autogenerated>:3
 8  0x000000000073a68d in github.com/pkg/sftp.sendPacket
    at ./packet.go:130
 9  0x000000000073338c in github.com/pkg/sftp.(*conn).sendPacket
    at ./conn.go:31
10  0x00000000007374b4 in github.com/pkg/sftp.(*packetManager).maybeSendPackets
    at ./packet-manager.go:87

  - Client is sending a new packet and is waiting on Pipe's condvar for
    the server to read it, while holding clientConn mutex:

 5  0x0000000000464a4c in io.(*PipeWriter).Write
    at /usr/lib/go/src/io/pipe.go:157
 6  0x0000000000790cf6 in github.com/pkg/sftp.(*conn).Write
    at <autogenerated>:3
 7  0x000000000073a68d in github.com/pkg/sftp.sendPacket
    at ./packet.go:130
 8  0x000000000073338c in github.com/pkg/sftp.(*conn).sendPacket
    at ./conn.go:31
 9  0x0000000000733bfb in github.com/pkg/sftp.(*clientConn).dispatchRequest
    at ./conn.go:105
10  0x0000000000733a0b in github.com/pkg/sftp.(*clientConn).sendPacket
    at ./conn.go:97

  - Client's receive loop is processing a reply from the server and is
    trying to acquire clientConn mutex:

4  0x000000000046108d in sync.(*Mutex).Lock
   at /usr/lib/go/src/sync/mutex.go:87
5  0x00000000007336cb in github.com/pkg/sftp.(*clientConn).recv
   at ./conn.go:69
6  0x000000000073350d in github.com/pkg/sftp.(*clientConn).loop
   at ./conn.go:49
7  0x000000000045ab51 in runtime.goexit
   at /usr/lib/go/src/runtime/asm_amd64.s:2197

In this scenario neither server nor client can progress. Fix this by
releasing clienConn mutex *before* trying to send the packet.

Signed-off-by: Pavel Borzenkov <pavel.borzenkov@gmail.com>
This commit is contained in:
Pavel Borzenkov 2017-04-05 09:53:22 +00:00 committed by John Eikenberry
parent 9aa225fc15
commit 506f3a7848
1 changed files with 3 additions and 1 deletions

View File

@ -102,11 +102,13 @@ func (c *clientConn) sendPacket(p idmarshaler) (byte, []byte, error) {
func (c *clientConn) dispatchRequest(ch chan<- result, p idmarshaler) {
c.Lock()
c.inflight[p.id()] = ch
c.Unlock()
if err := c.conn.sendPacket(p); err != nil {
c.Lock()
delete(c.inflight, p.id())
c.Unlock()
ch <- result{err: err}
}
c.Unlock()
}
// broadcastErr sends an error to all goroutines waiting for a response.