Skip to content

Commit b211d06

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 b211d06

8 files changed

Lines changed: 63 additions & 86 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.

internal/winredirect/types_windows.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,12 @@ package winredirect
33
// IOCTL codes matching the kernel driver definitions.
44
// CTL_CODE(FILE_DEVICE_NETWORK=0x12, function, METHOD_BUFFERED=0, FILE_ANY_ACCESS=0)
55
const (
6-
ioctlSetConfig = (0x00120000 | (0x800 << 2)) // IOCTL_WINREDIRECT_SET_CONFIG
7-
ioctlStart = (0x00120000 | (0x801 << 2)) // IOCTL_WINREDIRECT_START
8-
ioctlStop = (0x00120000 | (0x802 << 2)) // IOCTL_WINREDIRECT_STOP
9-
ioctlGetPending = (0x00120000 | (0x803 << 2)) // IOCTL_WINREDIRECT_GET_PENDING
10-
ioctlSetVerdict = (0x00120000 | (0x804 << 2)) // IOCTL_WINREDIRECT_SET_VERDICT
11-
ioctlGetFatalInfo = (0x00120000 | (0x805 << 2)) // IOCTL_WINREDIRECT_GET_FATAL_INFO
6+
ioctlSetConfig = (0x00120000 | (0x800 << 2)) // IOCTL_WINREDIRECT_SET_CONFIG
7+
ioctlStart = (0x00120000 | (0x801 << 2)) // IOCTL_WINREDIRECT_START
8+
ioctlStop = (0x00120000 | (0x802 << 2)) // IOCTL_WINREDIRECT_STOP
9+
ioctlGetPending = (0x00120000 | (0x803 << 2)) // IOCTL_WINREDIRECT_GET_PENDING
10+
ioctlSetVerdict = (0x00120000 | (0x804 << 2)) // IOCTL_WINREDIRECT_SET_VERDICT
11+
ioctlGetFatalInfo = (0x00120000 | (0x805 << 2)) // IOCTL_WINREDIRECT_GET_FATAL_INFO
1212
)
1313

1414
const (

internal/winsys/constants.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ var FWPM_LAYER_ALE_CONNECT_REDIRECT_V6 = windows.GUID{
152152
}
153153

154154
const (
155-
FWP_ACTION_FLAG_CALLOUT uint32 = 0x00004000
155+
FWP_ACTION_FLAG_CALLOUT uint32 = 0x00004000
156156
FWP_ACTION_CALLOUT_TERMINATING uint32 = (0x00000003 | FWP_ACTION_FLAG_CALLOUT | FWP_ACTION_FLAG_TERMINATING)
157157
)
158158

redirect_nftables_rules.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,8 @@ import (
1212
"github.com/sagernet/nftables/expr"
1313
"github.com/sagernet/nftables/userdata"
1414
"github.com/sagernet/sing/common"
15-
"github.com/sagernet/sing/common/ranges"
16-
1715
E "github.com/sagernet/sing/common/exceptions"
16+
"github.com/sagernet/sing/common/ranges"
1817

1918
"golang.org/x/exp/slices"
2019
"golang.org/x/sys/unix"

redirect_route_linux.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,9 @@ import (
88
"net/netip"
99

1010
"github.com/sagernet/netlink"
11-
E "github.com/sagernet/sing/common/exceptions"
1211
"github.com/sagernet/sing/common"
1312
"github.com/sagernet/sing/common/control"
13+
E "github.com/sagernet/sing/common/exceptions"
1414

1515
"golang.org/x/sys/unix"
1616
)

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: 27 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -21,25 +21,23 @@ 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

2827
type autoRedirect struct {
29-
tunOptions *Options
30-
ctx context.Context
31-
connContext func(context.Context) context.Context
32-
handler Handler
33-
logger logger.Logger
34-
errorHandler func(error)
35-
networkMonitor NetworkUpdateMonitor
36-
networkListener *list.Element[NetworkUpdateCallback]
37-
interfaceFinder control.InterfaceFinder
38-
driverManager *winredirect.Manager
39-
redirectServer *redirectServer
40-
routeAddressSet *[]*netipx.IPSet
41-
routeExcludeAddressSet *[]*netipx.IPSet
42-
28+
tunOptions *Options
29+
ctx context.Context
30+
connContext func(context.Context) context.Context
31+
handler Handler
32+
logger logger.Logger
33+
errorHandler func(error)
34+
networkMonitor NetworkUpdateMonitor
35+
networkListener *list.Element[NetworkUpdateCallback]
36+
interfaceFinder control.InterfaceFinder
37+
driverManager *winredirect.Manager
38+
redirectServer *redirectServer
39+
40+
selfPID uint32
4341
enableIPv4 bool
4442
enableIPv6 bool
4543

@@ -56,22 +54,21 @@ type autoRedirect struct {
5654

5755
func NewAutoRedirect(options AutoRedirectOptions) (AutoRedirect, error) {
5856
r := &autoRedirect{
59-
tunOptions: options.TunOptions,
60-
ctx: options.Context,
61-
connContext: options.ConnContext,
62-
handler: options.Handler,
63-
logger: options.Logger,
64-
errorHandler: options.ErrorHandler,
65-
networkMonitor: options.NetworkMonitor,
66-
interfaceFinder: options.InterfaceFinder,
67-
routeAddressSet: options.RouteAddressSet,
68-
routeExcludeAddressSet: options.RouteExcludeAddressSet,
69-
workerCount: 1,
57+
tunOptions: options.TunOptions,
58+
ctx: options.Context,
59+
connContext: options.ConnContext,
60+
handler: options.Handler,
61+
logger: options.Logger,
62+
errorHandler: options.ErrorHandler,
63+
networkMonitor: options.NetworkMonitor,
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)