From 09268b375cbbe076ebccf64decb2b8ee304c6957 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Wed, 3 Jun 2026 16:09:03 +0000 Subject: [PATCH] device: avoid cycle-leaky runtime.SetFinalizer when unnecessary In tailscale/wireguard-go#65, @lkosewsk reproduced a memory leak seen in prod with lots of wireguard-go instances being created and destroyed, where they were still being retained forever due to cycles in the runtime.SetFinalizer reference graph. Really we shouldn't be using runtime.SetFinalizer anywhere. But we still use it on mobile platforms in WaitPool. But those platforms don't have thousands of tsnet.Server instances coming & going, so this is a half fix: avoid the finalizer registration on Linux, etc where the queue doesn't need to be drained and there's no WaitPool accounting. Just let GC handle it, without adding finalizer cycle complexity. Updates tailscale/corp#42776 Signed-off-by: Brad Fitzpatrick --- device/channels.go | 16 ++++++++++++++-- device/pools.go | 4 ++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/device/channels.go b/device/channels.go index 1eaec56..45b2a76 100644 --- a/device/channels.go +++ b/device/channels.go @@ -83,10 +83,16 @@ func newAutodrainingInboundQueue(device *Device) *autodrainingInboundQueue { q := &autodrainingInboundQueue{ c: make(chan *QueueInboundElementsContainer, QueueInboundSize), } - runtime.SetFinalizer(q, device.flushInboundQueue) + if device.needsInboundQueueFinalizer() { + runtime.SetFinalizer(q, device.flushInboundQueue) + } return q } +func (device *Device) needsInboundQueueFinalizer() bool { + return device.pool.messageBuffers.hasAccounting() +} + func (device *Device) flushInboundQueue(q *autodrainingInboundQueue) { for { select { @@ -116,10 +122,16 @@ func newAutodrainingOutboundQueue(device *Device) *autodrainingOutboundQueue { q := &autodrainingOutboundQueue{ c: make(chan *QueueOutboundElementsContainer, QueueOutboundSize), } - runtime.SetFinalizer(q, device.flushOutboundQueue) + if device.needsOutboundQueueFinalizer() { + runtime.SetFinalizer(q, device.flushOutboundQueue) + } return q } +func (device *Device) needsOutboundQueueFinalizer() bool { + return device.pool.messageBuffers.hasAccounting() +} + func (device *Device) flushOutboundQueue(q *autodrainingOutboundQueue) { for { select { diff --git a/device/pools.go b/device/pools.go index b7536a3..173486e 100644 --- a/device/pools.go +++ b/device/pools.go @@ -25,6 +25,10 @@ func NewWaitPool(max uint32, new func() any) *WaitPool { return p } +func (p *WaitPool) hasAccounting() bool { + return p != nil && p.max != 0 +} + func (p *WaitPool) Get() any { if p.max != 0 { p.lock.Lock()