device: convert runtime.SetFinalizer to AddCleanup (#71)
In PR #66, we tried to address a memory leak by avoiding runtime.SetFinalizer for autodrainingInboundQueue and autodrainingOutboundQueue unless there was something to do. However, when there is work to be done, these finalizers still leak memory because they’re still holding on to a cyclical reference to q. This applies to any platform that relies on a bounded device.WaitPool, like Android and iOS which both declare PreallocatedBuffersPerPool. This patch converts this logic to runtime.AddCleanup which is designed to avoid this problem. Updates tailscale/corp#42776 Signed-off-by: Simon Law <sfllaw@tailscale.com>
This commit is contained in:
parent
2ad9837e6c
commit
cd7ac13b86
2 changed files with 8 additions and 8 deletions
|
|
@ -84,7 +84,7 @@ func newAutodrainingInboundQueue(device *Device) *autodrainingInboundQueue {
|
||||||
c: make(chan *QueueInboundElementsContainer, QueueInboundSize),
|
c: make(chan *QueueInboundElementsContainer, QueueInboundSize),
|
||||||
}
|
}
|
||||||
if device.needsInboundQueueFinalizer() {
|
if device.needsInboundQueueFinalizer() {
|
||||||
runtime.SetFinalizer(q, device.flushInboundQueue)
|
runtime.AddCleanup(q, device.flushInboundQueue, q.c)
|
||||||
}
|
}
|
||||||
return q
|
return q
|
||||||
}
|
}
|
||||||
|
|
@ -93,10 +93,10 @@ func (device *Device) needsInboundQueueFinalizer() bool {
|
||||||
return device.pool.messageBuffers.hasAccounting()
|
return device.pool.messageBuffers.hasAccounting()
|
||||||
}
|
}
|
||||||
|
|
||||||
func (device *Device) flushInboundQueue(q *autodrainingInboundQueue) {
|
func (device *Device) flushInboundQueue(c <-chan *QueueInboundElementsContainer) {
|
||||||
for {
|
for {
|
||||||
select {
|
select {
|
||||||
case elemsContainer := <-q.c:
|
case elemsContainer := <-c:
|
||||||
elemsContainer.filling.Wait()
|
elemsContainer.filling.Wait()
|
||||||
for _, elem := range elemsContainer.elems {
|
for _, elem := range elemsContainer.elems {
|
||||||
device.PutMessageBuffer(elem.buffer)
|
device.PutMessageBuffer(elem.buffer)
|
||||||
|
|
@ -123,7 +123,7 @@ func newAutodrainingOutboundQueue(device *Device) *autodrainingOutboundQueue {
|
||||||
c: make(chan *QueueOutboundElementsContainer, QueueOutboundSize),
|
c: make(chan *QueueOutboundElementsContainer, QueueOutboundSize),
|
||||||
}
|
}
|
||||||
if device.needsOutboundQueueFinalizer() {
|
if device.needsOutboundQueueFinalizer() {
|
||||||
runtime.SetFinalizer(q, device.flushOutboundQueue)
|
runtime.AddCleanup(q, device.flushOutboundQueue, q.c)
|
||||||
}
|
}
|
||||||
return q
|
return q
|
||||||
}
|
}
|
||||||
|
|
@ -132,10 +132,10 @@ func (device *Device) needsOutboundQueueFinalizer() bool {
|
||||||
return device.pool.messageBuffers.hasAccounting()
|
return device.pool.messageBuffers.hasAccounting()
|
||||||
}
|
}
|
||||||
|
|
||||||
func (device *Device) flushOutboundQueue(q *autodrainingOutboundQueue) {
|
func (device *Device) flushOutboundQueue(c <-chan *QueueOutboundElementsContainer) {
|
||||||
for {
|
for {
|
||||||
select {
|
select {
|
||||||
case elemsContainer := <-q.c:
|
case elemsContainer := <-c:
|
||||||
elemsContainer.filling.Wait()
|
elemsContainer.filling.Wait()
|
||||||
for _, elem := range elemsContainer.elems {
|
for _, elem := range elemsContainer.elems {
|
||||||
device.PutOutboundBuffer(elem.buffer)
|
device.PutOutboundBuffer(elem.buffer)
|
||||||
|
|
|
||||||
|
|
@ -244,8 +244,8 @@ func (peer *Peer) Start() {
|
||||||
|
|
||||||
peer.timersStart()
|
peer.timersStart()
|
||||||
|
|
||||||
device.flushInboundQueue(peer.queue.inbound)
|
device.flushInboundQueue(peer.queue.inbound.c)
|
||||||
device.flushOutboundQueue(peer.queue.outbound)
|
device.flushOutboundQueue(peer.queue.outbound.c)
|
||||||
|
|
||||||
// Use the device batch size, not the bind batch size, as the device size is
|
// Use the device batch size, not the bind batch size, as the device size is
|
||||||
// the size of the batch pools.
|
// the size of the batch pools.
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue