forked from quic-go/quic-go
return stream frames to pool on stream cancellations (#5327)
* fix: return stream frames to pool on error paths fixes potential memory leak where StreamFrames were not returned to sync.Pool in error/cancellation paths. root cause: frames are llocated via GetStreamFrame() but never returned via PutBack() when streams enter terminal states without going through normal OnAcked() cleanup. fixed paths: - CancelWrite(): return frames when reliableOffset == 0 - handleStopSendingFrame(): return frames on STOP_SENDING - closeForShutdown(): return frames on connection shutdown - OnLost(): return frame when stream reset with no data sent * fix: clear slice before nil assignment, remove excessive tests - add clear() call to properly release GC references per https://github.com/quic-go/quic-go/pull/5327#discussion_r2328451862 - remove excessive pool return tests per https://github.com/quic-go/quic-go/pull/5327#discussion_r2328453227
This commit is contained in:
@@ -454,6 +454,19 @@ func (s *SendStream) SetReliableBoundary() {
|
||||
}
|
||||
}
|
||||
|
||||
// returnFramesToPool returns all queued frames to the sync.Pool
|
||||
func (s *SendStream) returnFramesToPool() {
|
||||
for _, f := range s.retransmissionQueue {
|
||||
f.PutBack()
|
||||
}
|
||||
clear(s.retransmissionQueue)
|
||||
s.retransmissionQueue = nil
|
||||
if s.nextFrame != nil {
|
||||
s.nextFrame.PutBack()
|
||||
s.nextFrame = nil
|
||||
}
|
||||
}
|
||||
|
||||
// CancelWrite aborts sending on this stream.
|
||||
// Data already written, but not yet delivered to the peer is not guaranteed to be delivered reliably.
|
||||
// Write will unblock immediately, and future calls to Write will fail.
|
||||
@@ -485,7 +498,7 @@ func (s *SendStream) CancelWrite(errorCode StreamErrorCode) {
|
||||
reliableOffset := s.reliableOffset()
|
||||
if reliableOffset == 0 {
|
||||
s.numOutstandingFrames = 0
|
||||
s.retransmissionQueue = nil
|
||||
s.returnFramesToPool()
|
||||
}
|
||||
s.queuedResetStreamFrame = &wire.ResetStreamFrame{
|
||||
StreamID: s.streamID,
|
||||
@@ -561,7 +574,7 @@ func (s *SendStream) handleStopSendingFrame(f *wire.StopSendingFrame) {
|
||||
// if the peer stopped reading from the stream, there's no need to transmit any data reliably
|
||||
s.reliableSize = 0
|
||||
s.numOutstandingFrames = 0
|
||||
s.retransmissionQueue = nil
|
||||
s.returnFramesToPool()
|
||||
if s.resetErr == nil {
|
||||
s.resetErr = &StreamError{StreamID: s.streamID, ErrorCode: f.ErrorCode, Remote: true}
|
||||
s.ctxCancel(s.resetErr)
|
||||
@@ -629,6 +642,7 @@ func (s *SendStream) closeForShutdown(err error) {
|
||||
s.mutex.Lock()
|
||||
if s.shutdownErr == nil && !s.finishedWriting {
|
||||
s.shutdownErr = err
|
||||
s.returnFramesToPool()
|
||||
}
|
||||
s.mutex.Unlock()
|
||||
s.signalWrite()
|
||||
@@ -673,6 +687,8 @@ func (s *sendStreamAckHandler) OnLost(f wire.Frame) {
|
||||
// If the reliable size was 0 when the stream was cancelled,
|
||||
// the number of outstanding frames was immediately set to 0, and the retransmission queue was dropped.
|
||||
if s.resetErr != nil && (*SendStream)(s).reliableOffset() == 0 {
|
||||
// Return the frame to pool since it won't be retransmitted
|
||||
sf.PutBack()
|
||||
s.mutex.Unlock()
|
||||
return
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user