Merge pull request #118 from embogomolov/fix/interrupt-group-deadlock-extended

common/interrupt: close connections outside group lock

README.md

@@ -3,7 +3,7 @@
 [![license](https://img.shields.io/badge/license-GPLv3-blue.svg)](LICENSE)
 [![go](https://img.shields.io/badge/go-1.26-00ADD8.svg)](go.mod)
 [![codeberg](https://img.shields.io/badge/mirror-codeberg-2185D0.svg)](https://codeberg.org/shtorm-7/sing-box-extended)
-[![telegram](https://img.shields.io/badge/telegram-chat-26A5E4.svg)](https://t.me/sing_box_extended)
+[![telegram](https://img.shields.io/badge/telegram-chat-26A5E4.svg?logo=telegram)](https://t.me/sing_box_extended)
 
 Sing-box with extended features.
 

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()
 }
 

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()
+	}
 }

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")
+		}
+	}
+}

transport/v2rayxhttp/client.go

@@ -348,6 +348,7 @@ func createHTTPClient(ctx context.Context, dest M.Socksaddr, dialer N.Dialer, op
 				}
 				conn, dErr := qtls.DialEarly(ctx, bufio.NewUnbindPacketConn(udpConn), udpConn.RemoteAddr(), tlsConfig, cfg)
 				if dErr != nil {
+					_ = udpConn.Close()
 					return nil, dErr
 				}
 				if congestionControlFactory != nil {