pion
ConnectionState changes may be reported incorrectly
I already reported this in pion/webrtc but it is bug in pion/ice, so I am adding it here too for visibility. Original issue: https://github.com/pion/webrtc/issues/2578
Sometimes ConnectionState changes may be reported incorrectly - instead of getting Checking first and then Connected, state changes first to Connected, then Checking. This problem is caused by fact that Agent.connectionStateRoutine creates new goroutine to send new state (https://github.com/pion/ice/blob/master/agent_handlers.go#L52). Sometimes goroutine created to send Checking state is not executed immediately, but after another one used to send Connected state. I tried to remove go keyword there and looks that it fixed this problem. To avoid potential problems elsewhere, it would be good to add some buffer to chanState channel too.
Great find @sirzooro!
I think we need to keep the goroutine, but deliver the events in order. If we drop the goroutine the user can block our main thread.
Maybe we can internally have a buffered channel and have a goroutine deliver them to the user?
If you are interested in implementing this I would love to work with you on it!
Yes, I can create PR for this.
connectionStateRoutine is started in separate goroutine, so adding buffer to chanState should be enough. Probably 10 entries would be enough.
BTW, chanCandidate and chanCandidatePair channels also do not have buffers - adding buffers of size 10 and 2 respectively probably would help them too.
great! Totally agree with that.
Could you write a test that blocks the callback with something long running? Just to confirm we aren't introducing a behavior change?