Skip to content

Commit 27c7699

Browse files
committed
fix(dns): prevent long-run dns stall with bounded async and transport cleanup
1 parent 79d29aa commit 27c7699

File tree

2 files changed

+64
-14
lines changed

2 files changed

+64
-14
lines changed

control/control_plane.go

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,12 @@ import (
4545
"golang.org/x/sys/unix"
4646
)
4747

48+
const (
49+
// Cap async DNS handling to avoid unbounded goroutine growth under
50+
// sustained high-rate DNS traffic.
51+
maxAsyncDnsInFlight = 512
52+
)
53+
4854
type ControlPlane struct {
4955
log *logrus.Logger
5056

@@ -58,6 +64,7 @@ type ControlPlane struct {
5864

5965
dnsController *DnsController
6066
onceNetworkReady sync.Once
67+
dnsAsyncSem chan struct{}
6168

6269
dialMode consts.DialMode
6370

@@ -390,6 +397,7 @@ func NewControlPlane(
390397
outbounds: outbounds,
391398
dnsController: nil,
392399
onceNetworkReady: sync.Once{},
400+
dnsAsyncSem: make(chan struct{}, maxAsyncDnsInFlight),
393401
dialMode: dialMode,
394402
routingMatcher: routingMatcher,
395403
ctx: ctx,
@@ -833,20 +841,33 @@ func (c *ControlPlane) Serve(readyChan chan<- bool, listener *Listener) (err err
833841
// DNS packets must not block the per-src serial task queue:
834842
// a single slow upstream (up to DefaultDialTimeout=8s) would
835843
// stall all subsequent packets from the same source IP.
836-
// DNS is stateless; parallel handling is safe.
844+
// DNS is stateless; parallel handling is safe. Use a bounded
845+
// semaphore to avoid unbounded goroutine growth.
837846
if pktDst.Port() == 53 || pktDst.Port() == 5353 {
838-
// Transfer buffer ownership to the goroutine; clear defers.
839-
gData := data
840-
gOob := oob
841-
data = nil
842-
oob = nil
843-
go func() {
844-
defer gData.Put()
845-
defer gOob.Put()
846-
if e := c.handlePkt(udpConn, gData, convergeSrc, common.ConvergeAddrPort(pktDst), common.ConvergeAddrPort(realDst), routingResult, false); e != nil {
847+
select {
848+
case c.dnsAsyncSem <- struct{}{}:
849+
// Transfer buffer ownership to the goroutine; clear defers.
850+
gData := data
851+
gOob := oob
852+
data = nil
853+
oob = nil
854+
go func() {
855+
defer func() {
856+
<-c.dnsAsyncSem
857+
}()
858+
defer gData.Put()
859+
defer gOob.Put()
860+
if e := c.handlePkt(udpConn, gData, convergeSrc, common.ConvergeAddrPort(pktDst), common.ConvergeAddrPort(realDst), routingResult, false); e != nil {
861+
c.log.Warnln("handlePkt(dns):", e)
862+
}
863+
}()
864+
default:
865+
// Semaphore saturated: fall back to sync handling here to
866+
// apply backpressure instead of spawning unbounded goroutines.
867+
if e := c.handlePkt(udpConn, data, convergeSrc, common.ConvergeAddrPort(pktDst), common.ConvergeAddrPort(realDst), routingResult, false); e != nil {
847868
c.log.Warnln("handlePkt(dns):", e)
848869
}
849-
}()
870+
}
850871
return
851872
}
852873
if e := c.handlePkt(udpConn, data, convergeSrc, common.ConvergeAddrPort(pktDst), common.ConvergeAddrPort(realDst), routingResult, false); e != nil {

control/dns.go

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,13 +76,29 @@ type DoH struct {
7676
client *http.Client
7777
}
7878

79+
func closeDoHClient(client *http.Client) error {
80+
if client == nil || client.Transport == nil {
81+
return nil
82+
}
83+
// HTTP/1.1 and HTTP/2 transport.
84+
if t, ok := client.Transport.(interface{ CloseIdleConnections() }); ok {
85+
t.CloseIdleConnections()
86+
}
87+
// HTTP/3 transport.
88+
if closer, ok := client.Transport.(io.Closer); ok {
89+
return closer.Close()
90+
}
91+
return nil
92+
}
93+
7994
func (d *DoH) ForwardDNS(ctx context.Context, data []byte) (*dnsmessage.Msg, error) {
8095
if d.client == nil {
8196
d.client = d.getClient()
8297
}
8398
msg, err := sendHttpDNS(ctx, d.client, d.dialArgument.bestTarget.String(), &d.Upstream, data)
8499
if err != nil {
85100
// If failed to send DNS request, we should try to create a new client.
101+
_ = closeDoHClient(d.client)
86102
d.client = d.getClient()
87103
msg, err = sendHttpDNS(ctx, d.client, d.dialArgument.bestTarget.String(), &d.Upstream, data)
88104
if err != nil {
@@ -155,7 +171,9 @@ func (d *DoH) getHttp3RoundTripper() *http3.RoundTripper {
155171
}
156172

157173
func (d *DoH) Close() error {
158-
return nil
174+
err := closeDoHClient(d.client)
175+
d.client = nil
176+
return err
159177
}
160178

161179
type DoQ struct {
@@ -165,6 +183,13 @@ type DoQ struct {
165183
connection quic.EarlyConnection
166184
}
167185

186+
func closeDoQConnection(conn quic.EarlyConnection) error {
187+
if conn == nil {
188+
return nil
189+
}
190+
return conn.CloseWithError(0, "")
191+
}
192+
168193
func (d *DoQ) ForwardDNS(ctx context.Context, data []byte) (*dnsmessage.Msg, error) {
169194
if d.connection == nil {
170195
qc, err := d.createConnection(ctx)
@@ -176,7 +201,9 @@ func (d *DoQ) ForwardDNS(ctx context.Context, data []byte) (*dnsmessage.Msg, err
176201

177202
stream, err := d.connection.OpenStreamSync(ctx)
178203
if err != nil {
179-
// If failed to open stream, we should try to create a new connection.
204+
// If failed to open stream, close old connection and try to create a new one.
205+
_ = closeDoQConnection(d.connection)
206+
d.connection = nil
180207
qc, err := d.createConnection(ctx)
181208
if err != nil {
182209
return nil, err
@@ -230,7 +257,9 @@ func (d *DoQ) createConnection(ctx context.Context) (quic.EarlyConnection, error
230257
}
231258

232259
func (d *DoQ) Close() error {
233-
return nil
260+
err := closeDoQConnection(d.connection)
261+
d.connection = nil
262+
return err
234263
}
235264

236265
type DoTLS struct {

0 commit comments

Comments
 (0)