From 9acbc661f6217d64fbc2ed9141c4650f28bea020 Mon Sep 17 00:00:00 2001 From: Joe Turki Date: Fri, 19 Sep 2025 05:42:41 +0300 Subject: [PATCH] Cleanup statsGetter after peer is closed --- interceptor.go | 5 +++++ interceptor_test.go | 23 +++++++++++++++++++++++ peerconnection.go | 3 +++ 3 files changed, 31 insertions(+) diff --git a/interceptor.go b/interceptor.go index 41041cb9..a70cd111 100644 --- a/interceptor.go +++ b/interceptor.go @@ -69,6 +69,11 @@ func lookupStats(id string) (stats.Getter, bool) { return nil, false } +// cleanupStats removes the stats getter for a given peerconnection.statsId. +func cleanupStats(id string) { + statsGetter.Delete(id) +} + // key: string (peerconnection.statsId), value: stats.Getter var statsGetter sync.Map // nolint:gochecknoglobals diff --git a/interceptor_test.go b/interceptor_test.go index 8453b106..987b62f0 100644 --- a/interceptor_test.go +++ b/interceptor_test.go @@ -384,6 +384,29 @@ func TestStatsInterceptorIsAddedByDefault(t *testing.T) { ) } +// TestStatsGetterCleanup tests that statsGetter is properly cleaned up to prevent memory leaks. +func TestStatsGetterCleanup(t *testing.T) { + api := NewAPI() + pc, err := api.NewPeerConnection(Configuration{}) + assert.NoError(t, err) + + assert.NotNil(t, pc.statsGetter, "statsGetter should be non-nil after creation") + + statsID := pc.statsID + getter, exists := lookupStats(statsID) + assert.True(t, exists, "global statsGetter map should contain entry for this PC") + assert.NotNil(t, getter, "looked up getter should not be nil") + assert.Equal(t, pc.statsGetter, getter, "field and global map getter should match") + + assert.NoError(t, pc.Close()) + + assert.Nil(t, pc.statsGetter, "statsGetter field should be nil after close") + + getter, exists = lookupStats(statsID) + assert.False(t, exists, "global statsGetter map should not contain entry after close") + assert.Nil(t, getter, "looked up getter should be nil after close") +} + // TestInterceptorNack is an end-to-end test for the NACK sender. // It tests that: // - we get a NACK if we negotiated generic NACks; diff --git a/peerconnection.go b/peerconnection.go index 0d761c06..78244965 100644 --- a/peerconnection.go +++ b/peerconnection.go @@ -2481,6 +2481,9 @@ func (pc *PeerConnection) close(shouldGracefullyClose bool) error { //nolint:cyc closeErrs = append(closeErrs, doGracefulCloseOps()...) //nolint:makezero // todo fix + pc.statsGetter = nil + cleanupStats(pc.statsID) + // Interceptor closes at the end to prevent Bind from being called after interceptor is closed closeErrs = append(closeErrs, pc.api.interceptor.Close()) //nolint:makezero // todo fix