diff --git a/common/interrupt/conn.go b/common/interrupt/conn.go index b8235d6a..b3f1d5ec 100644 --- a/common/interrupt/conn.go +++ b/common/interrupt/conn.go @@ -29,8 +29,8 @@ type Conn struct { func (c *Conn) Close() error { c.group.access.Lock() - defer c.group.access.Unlock() c.group.connections.Remove(c.element) + c.group.access.Unlock() return c.Conn.Close() } @@ -58,8 +58,8 @@ type PacketConn struct { func (c *PacketConn) Close() error { c.group.access.Lock() - defer c.group.access.Unlock() c.group.connections.Remove(c.element) + c.group.access.Unlock() return c.PacketConn.Close() } @@ -87,8 +87,8 @@ type SingPacketConn struct { func (c *SingPacketConn) Close() error { c.group.access.Lock() - defer c.group.access.Unlock() c.group.connections.Remove(c.element) + c.group.access.Unlock() return c.PacketConn.Close() } diff --git a/common/interrupt/group.go b/common/interrupt/group.go index ae9095f8..bda30beb 100644 --- a/common/interrupt/group.go +++ b/common/interrupt/group.go @@ -47,15 +47,19 @@ func (g *Group) NewSingPacketConn(conn N.PacketConn, isExternal bool, isProvider func (g *Group) Interrupt(interruptExternalConnections bool) { g.access.Lock() - defer g.access.Unlock() var toDelete []*list.Element[*groupConnItem] + var toClose []io.Closer for element := g.connections.Front(); element != nil; element = element.Next() { if !element.Value.isExternal || interruptExternalConnections { - element.Value.conn.Close() toDelete = append(toDelete, element) + toClose = append(toClose, element.Value.conn) } } for _, element := range toDelete { g.connections.Remove(element) } + g.access.Unlock() + for _, conn := range toClose { + _ = conn.Close() + } } diff --git a/common/interrupt/group_test.go b/common/interrupt/group_test.go new file mode 100644 index 00000000..9cc10bf8 --- /dev/null +++ b/common/interrupt/group_test.go @@ -0,0 +1,66 @@ +package interrupt + +import ( + "net" + "sync" + "testing" + "time" +) + +type closeBarrierConn struct { + net.Conn + barrier *sync.WaitGroup +} + +func (c *closeBarrierConn) Close() error { + c.barrier.Done() + c.barrier.Wait() + return c.Conn.Close() +} + +func TestNestedGroupsInterruptWithoutDeadlock(t *testing.T) { + groupA := NewGroup() + groupB := NewGroup() + barrier := &sync.WaitGroup{} + barrier.Add(2) + + barrierA, barrierAPeer := net.Pipe() + barrierB, barrierBPeer := net.Pipe() + t.Cleanup(func() { + barrierAPeer.Close() + barrierBPeer.Close() + }) + groupA.NewConn(&closeBarrierConn{Conn: barrierA, barrier: barrier}, true, false) + groupB.NewConn(&closeBarrierConn{Conn: barrierB, barrier: barrier}, true, false) + + connA, connAPeer := net.Pipe() + connB, connBPeer := net.Pipe() + t.Cleanup(func() { + connAPeer.Close() + connBPeer.Close() + }) + wrapperA := groupA.NewConn(connA, true, false) + wrapperB := groupB.NewConn(connB, true, false) + groupA.NewConn(wrapperB, true, false) + groupB.NewConn(wrapperA, true, false) + + done := make(chan struct{}, 2) + go func() { + groupA.Interrupt(true) + done <- struct{}{} + }() + go func() { + groupB.Interrupt(true) + done <- struct{}{} + }() + + timeout := time.NewTimer(time.Second) + defer timeout.Stop() + for range 2 { + select { + case <-done: + case <-timeout.C: + t.Fatal("nested group interrupt deadlocked") + } + } +}