sctp icon indicating copy to clipboard operation
sctp copied to clipboard

Fix pendingBaseQueue memory

Open yann-y opened this issue 2 years ago • 2 comments

Description

The pendingBaseQueue array is converted to a list. When transferring large files, the array will not be gc, which takes up a lot of memory space.

yann-y avatar Feb 16 '23 10:02 yann-y

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.51 :tada:

Comparison is base (44ed465) 80.43% compared to head (bfc0280) 80.94%.

:exclamation: Current head bfc0280 differs from pull request most recent head cedecbc. Consider uploading reports for the commit cedecbc to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #264      +/-   ##
==========================================
+ Coverage   80.43%   80.94%   +0.51%     
==========================================
  Files          48       48              
  Lines        4043     4036       -7     
==========================================
+ Hits         3252     3267      +15     
+ Misses        648      628      -20     
+ Partials      143      141       -2     
Flag Coverage Δ
go 80.94% <100.00%> (+0.51%) :arrow_up:
wasm 67.49% <100.00%> (+0.43%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pending_queue.go 82.95% <100.00%> (+0.81%) :arrow_up:

... and 5 files with indirect coverage changes

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

codecov[bot] avatar Feb 16 '23 23:02 codecov[bot]

hey @yann-y just a heads up we are working on a v2 now that may obviate this issue, but wanted to keep you posted either way

edaniels avatar Feb 26 '24 13:02 edaniels

PendingQueue also leaks popped data, following change fixes it.

diff --git a/pending_queue.go b/pending_queue.go
index 6f70fea..44a956b 100644
--- a/pending_queue.go
+++ b/pending_queue.go
@@ -26,6 +26,7 @@ func (q *pendingBaseQueue) pop() *chunkPayloadData {
                return nil
        }
        c := q.queue[0]
+       q.queue[0] = nil
        q.queue = q.queue[1:]
        return c
 }

ukernel avatar Jan 22 '25 06:01 ukernel

PendingQueue also leaks popped data, following change fixes it.

@ukernel Thanks for finding this; a fix for this has been merged via #401. A release will be made soon which will include this change.

hey @yann-y just a heads up we are working on a v2 now that may obviate this issue, but wanted to keep you posted either way

@edaniels regarding your comment, I did want to note that I have picked up on the effort to update to v2 (see the parent issue: https://github.com/pion/sctp/issues/314) so hopefully the pending queue will be refactored + improved soon.

philipch07 avatar Oct 30 '25 16:10 philipch07