From b4a7b5a90818d6b8f61f297e06bed6674d12c547 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Tue, 1 Dec 2015 10:25:16 -0800 Subject: [PATCH 1/3] Fix close deadlock There was a race condition, where upon closing a conn and all its child streams, a (potential?) issue in the underlying transport library caused the 'Close' call to hang, leading to deadlocks in the peerstream code. This changes the lock to only be held during the closed'ness check, and should protect go-peerstream users against issues (misunderstandings?) in the underlying transport libs. --- conn.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/conn.go b/conn.go index 5482eaa..425f5f6 100644 --- a/conn.go +++ b/conn.go @@ -118,12 +118,12 @@ func (c *Conn) Streams() []*Stream { // Close closes this connection func (c *Conn) Close() error { c.closeLock.Lock() - defer c.closeLock.Unlock() - if c.closed { + c.closeLock.Unlock() return nil } c.closed = true + c.closeLock.Unlock() // close streams streams := c.Streams() From ff5f68c5f1c3477be90d0b0e5d8906f5f9c0007f Mon Sep 17 00:00:00 2001 From: Jeromy Date: Thu, 3 Dec 2015 12:20:00 -0800 Subject: [PATCH 2/3] add 'GoClose' method to help avoid deadlocks --- conn.go | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/conn.go b/conn.go index 425f5f6..16c18f9 100644 --- a/conn.go +++ b/conn.go @@ -47,6 +47,9 @@ type Conn struct { closed bool closeLock sync.Mutex + + closing bool + closingLock sync.Mutex } func newConn(nconn net.Conn, tconn smux.Conn, s *Swarm) *Conn { @@ -115,15 +118,32 @@ func (c *Conn) Streams() []*Stream { return streams } +// GoClose spawns off a goroutine to close the connection iff the connection is +// not already being closed and returns immediately +func (c *Conn) GoClose() { + c.closingLock.Lock() + defer c.closingLock.Unlock() + if c.closing { + return + } + c.closing = true + + go c.Close() +} + // Close closes this connection func (c *Conn) Close() error { c.closeLock.Lock() - if c.closed { - c.closeLock.Unlock() + defer c.closeLock.Unlock() + if c.closed == true { return nil } + + c.closingLock.Lock() + c.closing = true + c.closingLock.Unlock() + c.closed = true - c.closeLock.Unlock() // close streams streams := c.Streams() From 6f52ef1b40e19e902a7a1e5b24cbc72561dcc110 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Thu, 3 Dec 2015 12:29:28 -0800 Subject: [PATCH 3/3] use 'GoClose' in conns accessor --- swarm.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/swarm.go b/swarm.go index 4c4b6f9..1240fd3 100644 --- a/swarm.go +++ b/swarm.go @@ -184,7 +184,7 @@ func (s *Swarm) Conns() []*Conn { open := make([]*Conn, 0, len(conns)) for _, c := range conns { if c.smuxConn.IsClosed() { - c.Close() + c.GoClose() } else { open = append(open, c) }