Skip to content

Commit 72bb62a

Browse files
committed
windows: clean up dead code, comments, and fix goroutine leak
- Remove unused routeAddressSet/routeExcludeAddressSet fields and netipx import - Remove dead Metadata field from connEntry (already in Context) - Remove WHAT comments per project code-comment rules - Fix goroutine leak in connMetadataTable.cleanupLoop (add done channel) - Cache os.Getpid() as selfPID field to avoid repeated syscalls
1 parent 4aea1b6 commit 72bb62a

4 files changed

Lines changed: 37 additions & 59 deletions

File tree

internal/winipcfg/winipcfg.go

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,6 @@ func GetAnycastIPAddressTable(family AddressFamily) ([]MibAnycastIPAddressRow, e
134134
//
135135

136136
//sys getIPForwardTable2(family AddressFamily, table **mibIPforwardTable2) (ret error) = iphlpapi.GetIpForwardTable2
137-
//sys getBestRoute2(interfaceLUID *LUID, interfaceIndex uint32, sourceAddress *RawSockaddrInet, destinationAddress *RawSockaddrInet, sortOptions uint32, bestRoute *MibIPforwardRow2, bestSourceAddress *RawSockaddrInet) (ret error) = iphlpapi.GetBestRoute2
138137
//sys initializeIPForwardEntry(route *MibIPforwardRow2) = iphlpapi.InitializeIpForwardEntry
139138
//sys getIPForwardEntry2(route *MibIPforwardRow2) (ret error) = iphlpapi.GetIpForwardEntry2
140139
//sys setIPForwardEntry2(route *MibIPforwardRow2) (ret error) = iphlpapi.SetIpForwardEntry2
@@ -154,18 +153,6 @@ func GetIPForwardTable2(family AddressFamily) ([]MibIPforwardRow2, error) {
154153
return t, nil
155154
}
156155

157-
// GetBestRoute2 function retrieves the best route for the specified destination IP address on the local computer.
158-
// https://learn.microsoft.com/en-us/windows/win32/api/netioapi/nf-netioapi-getbestroute2
159-
func GetBestRoute2(interfaceLUID *LUID, interfaceIndex uint32, sourceAddress *RawSockaddrInet, destinationAddress *RawSockaddrInet, sortOptions uint32) (*MibIPforwardRow2, *RawSockaddrInet, error) {
160-
bestRoute := &MibIPforwardRow2{}
161-
bestSourceAddress := &RawSockaddrInet{}
162-
err := getBestRoute2(interfaceLUID, interfaceIndex, sourceAddress, destinationAddress, sortOptions, bestRoute, bestSourceAddress)
163-
if err != nil {
164-
return nil, nil, err
165-
}
166-
return bestRoute, bestSourceAddress, nil
167-
}
168-
169156
//
170157
// Notifications-related functions
171158
//

internal/winipcfg/zwinipcfg_windows.go

Lines changed: 0 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

redirect_server_windows.go

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@ func (s *redirectServer) Start() error {
5757

5858
func (s *redirectServer) Close() error {
5959
s.inShutdown.Store(true)
60+
if s.connTable != nil {
61+
s.connTable.Close()
62+
}
6063
if s.listener != nil {
6164
return s.listener.Close()
6265
}
@@ -104,12 +107,10 @@ func (s *redirectServer) loopIn() {
104107
}
105108
}
106109

107-
// connMetadataTable maps source address → connection metadata.
108-
// Entries are populated by pre-match workers before sending redirect verdicts,
109-
// and consumed by the redirect server upon accepting connections.
110110
type connMetadataTable struct {
111111
mu sync.Mutex
112112
entries map[connKey]*connEntry
113+
done chan struct{}
113114
}
114115

115116
type connKey struct {
@@ -120,7 +121,6 @@ type connKey struct {
120121
type connEntry struct {
121122
Destination M.Socksaddr
122123
Context context.Context
123-
Metadata *AutoRedirectMetadata
124124
IsDNS bool
125125
DNSServer M.Socksaddr
126126
CreatedAt time.Time
@@ -129,31 +129,38 @@ type connEntry struct {
129129
func newConnMetadataTable() *connMetadataTable {
130130
t := &connMetadataTable{
131131
entries: make(map[connKey]*connEntry),
132+
done: make(chan struct{}),
132133
}
133134
go t.cleanupLoop()
134135
return t
135136
}
136137

137-
func (t *connMetadataTable) Store(src M.Socksaddr, dst M.Socksaddr, ctx context.Context, metadata *AutoRedirectMetadata) {
138+
func (t *connMetadataTable) Close() {
139+
select {
140+
case <-t.done:
141+
default:
142+
close(t.done)
143+
}
144+
}
145+
146+
func (t *connMetadataTable) Store(src M.Socksaddr, dst M.Socksaddr, ctx context.Context) {
138147
key := connKey{Addr: src.Addr, Port: src.Port}
139148
t.mu.Lock()
140149
defer t.mu.Unlock()
141150
t.entries[key] = &connEntry{
142151
Destination: dst,
143152
Context: ctx,
144-
Metadata: metadata,
145153
CreatedAt: time.Now(),
146154
}
147155
}
148156

149-
func (t *connMetadataTable) StoreDNS(src M.Socksaddr, originalDst M.Socksaddr, dnsServer M.Socksaddr, ctx context.Context, metadata *AutoRedirectMetadata) {
157+
func (t *connMetadataTable) StoreDNS(src M.Socksaddr, originalDst M.Socksaddr, dnsServer M.Socksaddr, ctx context.Context) {
150158
key := connKey{Addr: src.Addr, Port: src.Port}
151159
t.mu.Lock()
152160
defer t.mu.Unlock()
153161
t.entries[key] = &connEntry{
154162
Destination: originalDst,
155163
Context: ctx,
156-
Metadata: metadata,
157164
IsDNS: true,
158165
DNSServer: dnsServer,
159166
CreatedAt: time.Now(),
@@ -192,14 +199,19 @@ func (t *connMetadataTable) Lookup(src M.Socksaddr) (*connEntry, bool) {
192199
func (t *connMetadataTable) cleanupLoop() {
193200
ticker := time.NewTicker(10 * time.Second)
194201
defer ticker.Stop()
195-
for range ticker.C {
196-
t.mu.Lock()
197-
now := time.Now()
198-
for key, entry := range t.entries {
199-
if now.Sub(entry.CreatedAt) > 30*time.Second {
200-
delete(t.entries, key)
202+
for {
203+
select {
204+
case <-t.done:
205+
return
206+
case <-ticker.C:
207+
t.mu.Lock()
208+
now := time.Now()
209+
for key, entry := range t.entries {
210+
if now.Sub(entry.CreatedAt) > 30*time.Second {
211+
delete(t.entries, key)
212+
}
201213
}
214+
t.mu.Unlock()
202215
}
203-
t.mu.Unlock()
204216
}
205217
}

redirect_windows.go

Lines changed: 10 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import (
2121
M "github.com/sagernet/sing/common/metadata"
2222
"github.com/sagernet/sing/common/x/list"
2323

24-
"go4.org/netipx"
2524
"golang.org/x/sys/windows"
2625
)
2726

@@ -35,11 +34,10 @@ type autoRedirect struct {
3534
networkMonitor NetworkUpdateMonitor
3635
networkListener *list.Element[NetworkUpdateCallback]
3736
interfaceFinder control.InterfaceFinder
38-
driverManager *winredirect.Manager
39-
redirectServer *redirectServer
40-
routeAddressSet *[]*netipx.IPSet
41-
routeExcludeAddressSet *[]*netipx.IPSet
37+
driverManager *winredirect.Manager
38+
redirectServer *redirectServer
4239

40+
selfPID uint32
4341
enableIPv4 bool
4442
enableIPv6 bool
4543

@@ -63,15 +61,14 @@ func NewAutoRedirect(options AutoRedirectOptions) (AutoRedirect, error) {
6361
logger: options.Logger,
6462
errorHandler: options.ErrorHandler,
6563
networkMonitor: options.NetworkMonitor,
66-
interfaceFinder: options.InterfaceFinder,
67-
routeAddressSet: options.RouteAddressSet,
68-
routeExcludeAddressSet: options.RouteExcludeAddressSet,
69-
workerCount: 1,
64+
interfaceFinder: options.InterfaceFinder,
65+
workerCount: 1,
7066
}
7167
return r, nil
7268
}
7369

7470
func (r *autoRedirect) Start() error {
71+
r.selfPID = uint32(os.Getpid())
7572
r.enableIPv4 = len(r.tunOptions.Inet4Address) > 0
7673
r.enableIPv6 = len(r.tunOptions.Inet6Address) > 0
7774
if !r.enableIPv4 && !r.enableIPv6 {
@@ -142,7 +139,7 @@ func (r *autoRedirect) startRedirect() error {
142139
redirectPort := M.AddrPortFromNet(server.listener.Addr()).Port()
143140
err = manager.SetConfig(&winredirect.Config{
144141
RedirectPort: redirectPort,
145-
ProxyPID: uint32(os.Getpid()),
142+
ProxyPID: r.selfPID,
146143
TunGUID: tunGUID,
147144
})
148145
if err != nil {
@@ -173,9 +170,6 @@ func (r *autoRedirect) Close() error {
173170
}
174171

175172
func (r *autoRedirect) UpdateRouteAddressSet() {
176-
// Dynamic route address sets are updated via pointer indirection.
177-
// The IPSet pointers are swapped atomically by the caller.
178-
// No driver communication needed — all filtering is in Go.
179173
}
180174

181175
func (r *autoRedirect) preMatchWorker() {
@@ -233,38 +227,33 @@ func (r *autoRedirect) evaluateConnection(conn *winredirect.PendingConn) uint32
233227
src := pendingConnSrc(conn)
234228

235229
// Proxy process outbound connections must never be redirected back into itself.
236-
if conn.ProcessID == uint32(os.Getpid()) {
230+
if conn.ProcessID == r.selfPID {
237231
return winredirect.VerdictBypass
238232
}
239233

240-
// 1. Loopback destinations
241234
if dst.Addr.IsLoopback() {
242235
return winredirect.VerdictBypass
243236
}
244237

245-
// DNS hijack: port 53 from local network → redirect to DNS server
246238
if !r.tunOptions.EXP_DisableDNSHijack && dst.Port == 53 {
247239
if r.isLocalAddress(src.Addr) {
248240
dnsServer := r.dnsServerForFamily(dst.Addr)
249241
if dnsServer.IsValid() {
250242
metadata := r.resolveMetadata(conn)
251243
ctx := r.newConnContext(metadata)
252-
r.redirectServer.connTable.StoreDNS(src, dst, M.SocksaddrFrom(dnsServer, 53), ctx, metadata)
244+
r.redirectServer.connTable.StoreDNS(src, dst, M.SocksaddrFrom(dnsServer, 53), ctx)
253245
return winredirect.VerdictRedirect
254246
}
255247
}
256248
}
257249

258-
// Strict route: reject disabled address family
259250
if r.tunOptions.StrictRoute && r.isDisabledFamily(dst.Addr) {
260251
return winredirect.VerdictDrop
261252
}
262253

263-
// Resolve PID → process path
264254
metadata := r.resolveMetadata(conn)
265255
ctx := r.newConnContext(metadata)
266256

267-
// PrepareConnection (NFQUEUE equivalent)
268257
_, err := r.handler.PrepareConnection(ctx, "tcp", src, dst, nil, 0)
269258
if errors.Is(err, ErrDrop) {
270259
return winredirect.VerdictDrop
@@ -276,8 +265,7 @@ func (r *autoRedirect) evaluateConnection(conn *winredirect.PendingConn) uint32
276265
r.logger.Warn("prepare connection fallback to redirect: ", err)
277266
}
278267

279-
// Store metadata for redirect server
280-
r.redirectServer.connTable.Store(src, dst, ctx, metadata)
268+
r.redirectServer.connTable.Store(src, dst, ctx)
281269

282270
return winredirect.VerdictRedirect
283271
}

0 commit comments

Comments
 (0)