Skip to content

Commit

Permalink
Fix deadlock when using Client + Service over io.Pipe
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
pborzenkov authored and eikenb committed Apr 5, 2017
1 parent 9aa225f commit 506f3a7
Showing 1 changed file with 3 additions and 1 deletion.
4 changes: 3 additions & 1 deletion conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 506f3a7

Please sign in to comment.