Close PeerConnection on DTLS CloseNotify

[Sean-Der]

Resolves #1767
Resolves pion/dtls#151

[codecov]

Codecov Report

Patch coverage is 71.42% of modified lines.

Files Changed	Coverage
peerconnection.go	57.14%
internal/mux/endpoint.go	83.33%
dtlstransport.go	100.00%

📢 Thoughts on this report? Let us know!.

[Sean-Der]

Would love your take @boks1971 @cnderrauber

Everyone should see much faster hang ups and should more closely match browser behavior. I hope I am not making any assumptions though.

[cnderrauber]

Looks good!

[boks1971]

@Sean-Der This is great. LGTM!

One curiosity question: I could not quite understand the test you added, i. e. how does it trigger the new path? It is basically the new path is going to be triggered pretty much most of the time? If yes, is there is a need for a new test (guessing existing tests would cover it).

[Sean-Der]

@boks1971 The test doesn’t explicitly close both PeerConnections. It then uses the ‘leaked threads checker’ so it can’t complete until both close.

I bet the new path is happening in most tests! As soon as you close a PeerConnection it sends the Close. It’s a race between that and closing both connections in each test :)

[boks1971]

@boks1971 The test doesn’t explicitly close both PeerConnections. It then uses the ‘leaked threads checker’ so it can’t complete until both close.

I bet the new path is happening in most tests! As soon as you close a PeerConnection it sends the Close. It’s a race between that and closing both connections in each test :)

aah, got it, thank you Sean.