fix: whep gathering failure leaks peer connections

[RouquinBlanc]

Tentative fix for #3118

It still leaks, but less; Here is what remains leaking:

goroutine profile: total 128
50 @ 0x100843218 0x10083cae8 0x100876730 0x1008b8538 0x1008b9880 0x1008b9871 0x100a20cb8 0x100a32234 0x100b5ddd0 0x100ac7488 0x100ac75f0 0x100b5ed58 0x10087d084
#   0x10087672f internal/poll.runtime_pollWait+0x9f     /opt/homebrew/opt/go/libexec/src/runtime/netpoll.go:345
#   0x1008b8537 internal/poll.(*pollDesc).wait+0x27     /opt/homebrew/opt/go/libexec/src/internal/poll/fd_poll_runtime.go:84
#   0x1008b987f internal/poll.(*pollDesc).waitRead+0x1ff    /opt/homebrew/opt/go/libexec/src/internal/poll/fd_poll_runtime.go:89
#   0x1008b9870 internal/poll.(*FD).Read+0x1f0          /opt/homebrew/opt/go/libexec/src/internal/poll/fd_unix.go:164
#   0x100a20cb7 net.(*netFD).Read+0x27              /opt/homebrew/opt/go/libexec/src/net/fd_posix.go:55
#   0x100a32233 net.(*conn).Read+0x33               /opt/homebrew/opt/go/libexec/src/net/net.go:179
#   0x100b5ddcf net/http.(*persistConn).Read+0x4f       /opt/homebrew/opt/go/libexec/src/net/http/transport.go:1976
#   0x100ac7487 bufio.(*Reader).fill+0xf7           /opt/homebrew/opt/go/libexec/src/bufio/bufio.go:110
#   0x100ac75ef bufio.(*Reader).Peek+0x5f           /opt/homebrew/opt/go/libexec/src/bufio/bufio.go:148
#   0x100b5ed57 net/http.(*persistConn).readLoop+0x157      /opt/homebrew/opt/go/libexec/src/net/http/transport.go:2140

50 @ 0x100843218 0x1008566b8 0x100b60600 0x10087d084
#   0x100b605ff net/http.(*persistConn).writeLoop+0x9f  /opt/homebrew/opt/go/libexec/src/net/http/transport.go:2443

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 42.30769% with 15 lines in your changes are missing coverage. Please review.

Project coverage is 55.57%. Comparing base (f8d2343) to head (fa24957).

Files	Patch %	Lines
internal/protocols/webrtc/whip_client.go	0.00%	15 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #3124 +/- ## ========================================== - Coverage 55.57% 55.57% -0.01% ========================================== Files 150 150 Lines 16822 16839 +17 ========================================== + Hits 9349 9358 +9 - Misses 6735 6744 +9 + Partials 738 737 -1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[RouquinBlanc]

~Second part seems to do the trick!~

Still leaking... Same thing, it's just slow to build up

[RouquinBlanc]

Seems fine now (running 20 minutes and no increase).

It looks like the HTTPClient keeps idle open connections for future requests and does not clean them up unless CloseIdleConnections() is called. They would eventually die out, but that depends on GC and/or remote side closing, which is not ideal.

🤞

[aler9]

Thanks for this patch, i did the following changes:

• i moved http.Client.CloseIdleConnections() outside WHIPClient, since the HTTP Client is not meant as a managed component of WHIPClient but as an external component that may be shared with other entities, therefore WHIPClient should not perform actions that may interfere with other entities
• i edited WHIPClient.Publish() and WHIPClient.Read() in order to automatically call WHIPClient.Close(), therefore calling it manually should not be necessary anymore

I agree with you on the fact that http.Client.CloseIdleConnections() should be called whenever a http.Client is not in use anymore, i'll edit the server in order to add this in all other places where http.Client is in use.

This has been merged, thanks!

[github-actions[bot]]

This issue is mentioned in release v1.7.0 🚀 Check out the entire changelog by clicking here