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 <bradfitz@tailscale.com>
This commit is contained in:
parent
010dd5c6f2
commit
09268b375c
2 changed files with 18 additions and 2 deletions
|
|
@ -83,10 +83,16 @@ func newAutodrainingInboundQueue(device *Device) *autodrainingInboundQueue {
|
|||
q := &autodrainingInboundQueue{
|
||||
c: make(chan *QueueInboundElementsContainer, QueueInboundSize),
|
||||
}
|
||||
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),
|
||||
}
|
||||
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 {
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue