From f29ef99b220beb906c60e7f1aebac6c02498daf9 Mon Sep 17 00:00:00 2001 From: Juliusz Chroboczek Date: Thu, 1 Aug 2024 10:39:30 +0200 Subject: [PATCH] Avoid leaking tickers In Go 1.22 and earlier, a ticker needs to be explicitly stopped when it's no longer useful in order to avoid a resource leak. In Go 1.23 and later, an orphaned ticker will eventually be garbage collected, but it's still more thrifty to stop it early. --- examples/bandwidth-estimation-from-disk/main.go | 1 + examples/data-channels-detach/jsfiddle/main.go | 4 +++- examples/data-channels-detach/main.go | 4 +++- examples/data-channels-flow-control/main.go | 4 +++- examples/data-channels/main.go | 4 +++- examples/insertable-streams/main.go | 3 ++- examples/ortc-media/main.go | 1 + examples/ortc/main.go | 4 +++- examples/pion-to-pion/answer/main.go | 4 +++- examples/pion-to-pion/offer/main.go | 4 +++- examples/play-from-disk-renegotiation/main.go | 1 + examples/play-from-disk/main.go | 2 ++ examples/simulcast/main.go | 1 + interceptor_test.go | 5 ++++- peerconnection_media_test.go | 1 + 15 files changed, 34 insertions(+), 9 deletions(-) diff --git a/examples/bandwidth-estimation-from-disk/main.go b/examples/bandwidth-estimation-from-disk/main.go index 54e73d1a00c..e1a12baf10b 100644 --- a/examples/bandwidth-estimation-from-disk/main.go +++ b/examples/bandwidth-estimation-from-disk/main.go @@ -194,6 +194,7 @@ func main() { // * avoids accumulating skew, just calling time.Sleep didn't compensate for the time spent parsing the data // * works around latency issues with Sleep (see https://github.com/golang/go/issues/44343) ticker := time.NewTicker(time.Millisecond * time.Duration((float32(header.TimebaseNumerator)/float32(header.TimebaseDenominator))*1000)) + defer ticker.Stop() frame := []byte{} frameHeader := &ivfreader.IVFFrameHeader{} currentTimestamp := uint64(0) diff --git a/examples/data-channels-detach/jsfiddle/main.go b/examples/data-channels-detach/jsfiddle/main.go index 6ed1626a899..a83755e1d46 100644 --- a/examples/data-channels-detach/jsfiddle/main.go +++ b/examples/data-channels-detach/jsfiddle/main.go @@ -159,7 +159,9 @@ func ReadLoop(d io.Reader) { // WriteLoop shows how to write to the datachannel directly func WriteLoop(d io.Writer) { - for range time.NewTicker(5 * time.Second).C { + ticker := time.NewTicker(5 * time.Second) + defer ticker.Stop() + for range ticker.C { message, err := randutil.GenerateCryptoRandomString(messageSize, "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") if err != nil { handleError(err) diff --git a/examples/data-channels-detach/main.go b/examples/data-channels-detach/main.go index 4a4ef2b3b7e..b7d8af52f48 100644 --- a/examples/data-channels-detach/main.go +++ b/examples/data-channels-detach/main.go @@ -150,7 +150,9 @@ func ReadLoop(d io.Reader) { // WriteLoop shows how to write to the datachannel directly func WriteLoop(d io.Writer) { - for range time.NewTicker(5 * time.Second).C { + ticker := time.NewTicker(5 * time.Second) + defer ticker.Stop() + for range ticker.C { message, err := randutil.GenerateCryptoRandomString(messageSize, "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") if err != nil { panic(err) diff --git a/examples/data-channels-flow-control/main.go b/examples/data-channels-flow-control/main.go index 0891ac71818..eed504dd006 100644 --- a/examples/data-channels-flow-control/main.go +++ b/examples/data-channels-flow-control/main.go @@ -113,7 +113,9 @@ func createAnswerer() *webrtc.PeerConnection { since := time.Now() // Start printing out the observed throughput - for range time.NewTicker(1000 * time.Millisecond).C { + ticker := time.NewTicker(1000 * time.Millisecond) + defer ticker.Stop() + for range ticker.C { bps := float64(atomic.LoadUint64(&totalBytesReceived)*8) / time.Since(since).Seconds() log.Printf("Throughput: %.03f Mbps", bps/1024/1024) } diff --git a/examples/data-channels/main.go b/examples/data-channels/main.go index 217f59f9dbc..75bf245e37f 100644 --- a/examples/data-channels/main.go +++ b/examples/data-channels/main.go @@ -70,7 +70,9 @@ func main() { d.OnOpen(func() { fmt.Printf("Data channel '%s'-'%d' open. Random messages will now be sent to any connected DataChannels every 5 seconds\n", d.Label(), d.ID()) - for range time.NewTicker(5 * time.Second).C { + ticker := time.NewTicker(5 * time.Second) + defer ticker.Stop() + for range ticker.C { message, sendErr := randutil.GenerateCryptoRandomString(15, "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") if sendErr != nil { panic(sendErr) diff --git a/examples/insertable-streams/main.go b/examples/insertable-streams/main.go index 65a43272925..9a8188b7cdf 100644 --- a/examples/insertable-streams/main.go +++ b/examples/insertable-streams/main.go @@ -89,7 +89,8 @@ func main() { // * avoids accumulating skew, just calling time.Sleep didn't compensate for the time spent parsing the data // * works around latency issues with Sleep (see https://github.com/golang/go/issues/44343) ticker := time.NewTicker(time.Millisecond * time.Duration((float32(header.TimebaseNumerator)/float32(header.TimebaseDenominator))*1000)) - for ; true; <-ticker.C { + defer ticker.Stop() + for range ticker.C { frame, _, ivfErr := ivf.ParseNextFrame() if errors.Is(ivfErr, io.EOF) { fmt.Printf("All frames parsed and sent") diff --git a/examples/ortc-media/main.go b/examples/ortc-media/main.go index f601280019e..3f187f033b8 100644 --- a/examples/ortc-media/main.go +++ b/examples/ortc-media/main.go @@ -223,6 +223,7 @@ func fourCCToTrack(fourCC string) *webrtc.TrackLocalStaticSample { // Write a file to Track func writeFileToTrack(ivf *ivfreader.IVFReader, header *ivfreader.IVFFileHeader, track *webrtc.TrackLocalStaticSample) { ticker := time.NewTicker(time.Millisecond * time.Duration((float32(header.TimebaseNumerator)/float32(header.TimebaseDenominator))*1000)) + defer ticker.Stop() for ; true; <-ticker.C { frame, _, err := ivf.ParseNextFrame() if errors.Is(err, io.EOF) { diff --git a/examples/ortc/main.go b/examples/ortc/main.go index 70098d0fad2..43d501d8824 100644 --- a/examples/ortc/main.go +++ b/examples/ortc/main.go @@ -183,7 +183,9 @@ func handleOnOpen(channel *webrtc.DataChannel) func() { return func() { fmt.Printf("Data channel '%s'-'%d' open. Random messages will now be sent to any connected DataChannels every 5 seconds\n", channel.Label(), channel.ID()) - for range time.NewTicker(5 * time.Second).C { + ticker := time.NewTicker(5 * time.Second) + defer ticker.Stop() + for range ticker.C { message, err := randutil.GenerateCryptoRandomString(15, "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") if err != nil { panic(err) diff --git a/examples/pion-to-pion/answer/main.go b/examples/pion-to-pion/answer/main.go index dd4dd7ab11e..e74427a4028 100644 --- a/examples/pion-to-pion/answer/main.go +++ b/examples/pion-to-pion/answer/main.go @@ -163,7 +163,9 @@ func main() { // nolint:gocognit d.OnOpen(func() { fmt.Printf("Data channel '%s'-'%d' open. Random messages will now be sent to any connected DataChannels every 5 seconds\n", d.Label(), d.ID()) - for range time.NewTicker(5 * time.Second).C { + ticker := time.NewTicker(5 * time.Second) + defer ticker.Stop() + for range ticker.C { message, sendTextErr := randutil.GenerateCryptoRandomString(15, "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") if sendTextErr != nil { panic(sendTextErr) diff --git a/examples/pion-to-pion/offer/main.go b/examples/pion-to-pion/offer/main.go index 16332f0d235..a299e5e92d7 100644 --- a/examples/pion-to-pion/offer/main.go +++ b/examples/pion-to-pion/offer/main.go @@ -144,7 +144,9 @@ func main() { //nolint:gocognit dataChannel.OnOpen(func() { fmt.Printf("Data channel '%s'-'%d' open. Random messages will now be sent to any connected DataChannels every 5 seconds\n", dataChannel.Label(), dataChannel.ID()) - for range time.NewTicker(5 * time.Second).C { + ticker := time.NewTicker(5 * time.Second) + defer ticker.Stop() + for range ticker.C { message, sendTextErr := randutil.GenerateCryptoRandomString(15, "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") if sendTextErr != nil { panic(sendTextErr) diff --git a/examples/play-from-disk-renegotiation/main.go b/examples/play-from-disk-renegotiation/main.go index 686b94b30b6..7e871f76cef 100644 --- a/examples/play-from-disk-renegotiation/main.go +++ b/examples/play-from-disk-renegotiation/main.go @@ -184,6 +184,7 @@ func writeVideoToTrack(t *webrtc.TrackLocalStaticSample) { // * avoids accumulating skew, just calling time.Sleep didn't compensate for the time spent parsing the data // * works around latency issues with Sleep (see https://github.com/golang/go/issues/44343) ticker := time.NewTicker(time.Millisecond * time.Duration((float32(header.TimebaseNumerator)/float32(header.TimebaseDenominator))*1000)) + defer ticker.Stop() for ; true; <-ticker.C { frame, _, err := ivf.ParseNextFrame() if err != nil { diff --git a/examples/play-from-disk/main.go b/examples/play-from-disk/main.go index c6bb33632ff..7b470427231 100644 --- a/examples/play-from-disk/main.go +++ b/examples/play-from-disk/main.go @@ -132,6 +132,7 @@ func main() { // * avoids accumulating skew, just calling time.Sleep didn't compensate for the time spent parsing the data // * works around latency issues with Sleep (see https://github.com/golang/go/issues/44343) ticker := time.NewTicker(time.Millisecond * time.Duration((float32(header.TimebaseNumerator)/float32(header.TimebaseDenominator))*1000)) + defer ticker.Stop() for ; true; <-ticker.C { frame, _, ivfErr := ivf.ParseNextFrame() if errors.Is(ivfErr, io.EOF) { @@ -197,6 +198,7 @@ func main() { // * avoids accumulating skew, just calling time.Sleep didn't compensate for the time spent parsing the data // * works around latency issues with Sleep (see https://github.com/golang/go/issues/44343) ticker := time.NewTicker(oggPageDuration) + defer ticker.Stop() for ; true; <-ticker.C { pageData, pageHeader, oggErr := ogg.ParseNextPage() if errors.Is(oggErr, io.EOF) { diff --git a/examples/simulcast/main.go b/examples/simulcast/main.go index 25b06928906..96dac358f79 100644 --- a/examples/simulcast/main.go +++ b/examples/simulcast/main.go @@ -113,6 +113,7 @@ func main() { rid := track.RID() go func() { ticker := time.NewTicker(3 * time.Second) + defer ticker.Stop() for range ticker.C { fmt.Printf("Sending pli for stream with rid: %q, ssrc: %d\n", track.RID(), track.SSRC()) if writeErr := peerConnection.WriteRTCP([]rtcp.Packet{&rtcp.PictureLossIndication{MediaSSRC: uint32(track.SSRC())}}); writeErr != nil { diff --git a/interceptor_test.go b/interceptor_test.go index 2670627df74..c7c3879d790 100644 --- a/interceptor_test.go +++ b/interceptor_test.go @@ -92,6 +92,7 @@ func TestPeerConnection_Interceptor(t *testing.T) { func() { ticker := time.NewTicker(time.Millisecond * 20) + defer ticker.Stop() for { select { case <-seenRTP.Done(): @@ -252,7 +253,9 @@ func Test_Interceptor_ZeroSSRC(t *testing.T) { go func() { sequenceNumber := uint16(0) - for range time.NewTicker(time.Millisecond * 20).C { + ticker := time.NewTicker(time.Millisecond * 20) + defer ticker.Stop() + for range ticker.C { track.mu.Lock() if len(track.bindings) == 1 { _, err = track.bindings[0].writeStream.WriteRTP(&rtp.Header{ diff --git a/peerconnection_media_test.go b/peerconnection_media_test.go index 121b41c68a1..8ee90a46a0e 100644 --- a/peerconnection_media_test.go +++ b/peerconnection_media_test.go @@ -1000,6 +1000,7 @@ func TestPeerConnection_Simulcast_Probe(t *testing.T) { assert.NoError(t, err) ticker := time.NewTicker(time.Millisecond * 20) + defer ticker.Stop() testFinished := make(chan struct{}) seenFiveStreams, seenFiveStreamsCancel := context.WithCancel(context.Background())