Repository navigation
Assert that OnICEConnectionStateChange doesn't incorrectly report disconnected #1002
Description
Your environment.
- Version: 20f2d18
- Browser: Chrome 78.0.3904.97
- Other Information - running Pion in a remote docker container.
What did you do?
Established a connection with a remote server.
What did you expect?
The peer to stay connected with the server until it decides to close the connection.
What happened?
On Chrome, the connection is being closed (going to a disconnected state) 30 seconds after being established.
After some debugging it seems this method is responsible for closing the connection:
https://github.com/pion/ice/blob/37bfc6f4d997e5a6d9e836fd9607b9c9aefbc2b6/agent.go#L769
Some lines that appear to be relevant from the debug log:
ice INFO: 2020/02/04 13:25:33 Setting new connection state: Checking
ice DEBUG: 13:25:33.768620 agent.go:1143: adding a new peer-reflexive candidate: $remote:43116
ice TRACE: 13:25:33.816489 agent.go:639: Set selected candidate pair: prio 7998392936314175487 (local, prio 2130706431) host $local:39823 <-> prflx $remote:43116 related :0 (remote, prio 1862270975)
ice INFO: 2020/02/04 13:25:33 Setting new connection state: Connected
ice DEBUG: 13:25:34.015233 agent.go:1143: adding a new peer-reflexive candidate: $remote:1619
ice DEBUG: 13:25:34.253331 agent.go:1143: adding a new peer-reflexive candidate: $remote:48095
ice DEBUG: 13:25:34.446345 agent.go:1143: adding a new peer-reflexive candidate: $remote:42324
....
ice INFO: 2020/02/04 13:26:05 Setting new connection state: Disconnected
ice INFO: 2020/02/04 13:26:05 Setting new connection state: Closed
In this example, after the call to adding a new peer-reflexive candidate: $remote:1619, I see no more traffic coming from $remote:43116 but I still see traffic coming from the other candidates.
The selected candidate pair candidate pair doesn't change though which results in the connection timing out.
While the connection is established I can receive the audio/video tracks in the browser as expected.
For some reason Firefox doesn't seem to have the same problem.
Albeit I'm seeing a single adding a new peer-reflexive candidate in that case 🤔
Any suggestions to further debug this?
Activity
gqgs commentedon Feb 4, 2020
Apparently our handling of the
Disconnectedstate was incorrect.The connection can still be reestablished after moving to that state.
There is still a disconnect the first time the client connects but I suppose that's expected for some reason?
I'm still wondering what would be the proper way of detecting a dead connection in this case since apparently we can't rely on the ICE connection state for that.
LanderN commentedon Jun 11, 2020
We have just encountered the same problem and after A LOT of debugging, we found this case happens when multiple ICE candidate pairs are marked as valid.
One of them goes to disconnected or failed, and because we hooked
OnConnectionStateChange(webrtc.ICEConnectionStateDisconnected) to our cleanup function, the connection was torn down.Is this a bug or just the weird/bad WebRTC API causing problems?
Sean-Der commentedon Jun 13, 2020
Oof sorry about that @LanderN
Let me write a test that asserts that
Disconnectedonly gets fired when ALL pairs are disconnected. When fully nominated/selected will also confirm that only that candidate pair effects the state. There is also the 'Fatal' state, when the ICE Agent is completely dead. We don't implement that yet pion/ice#189Disconnectedcould be temporarypion/webrtcwill try to reconnect so maybe I wouldn't hang up right away!Thanks for reporting the bug, and will get that fixed this weekend.
LanderN commentedon Jun 13, 2020
Hey Sean, that sounds great!
Reading the linked issue and the discussion around it, would you suggest a manual call to
peerConnection.Close()after the ConnectionState has gone tofailedor would that be unnecessary in the proposed implementation? I do have some other state attached to each connection that I would like to clean up when possible, I assume this should be dealt with on a change toclosed?[-]Connection disconnects 30 seconds after being established[/-][+]Assert that OnICEConnectionStateChange doesn't incorrectly report disconnected[/+]Sean-Der commentedon Oct 4, 2020
Sorry I haven't dealt with this yet :( I just haven't had the bandwidth.
If people are interested in contributing a simple test would be great here! Using
vnetestablish two valid candidate pairs, but then cause one to go to disconnected.You should assert that
webrtc.ICEConnectionStateDisconnectedis not fired. If that test fails we can go from there, should be easy enough to fix :)Sean-Der commentedon Oct 7, 2020
@rahulnakre @scorpionknifes if either of you are interested I think this would be a fun issue to look into! Even if this bug doesn't happen anymore this would be a great test to have :)
rahulnakre commentedon Oct 8, 2020
hey, I'll take a look when I get the chance, thanks!
bewa1t commentedon Apr 29, 2022
Hey, I just discovered the project, is there a chance that I can try to pick this up?
boushley commentedon May 1, 2022
Go for it @wtsiamruk! Sean is super friendly and open to community participation! Given that no one else has picked this one up for over a year I think you're good to give it a go.
Sean-Der commentedon Apr 29, 2024
Fixed by pion/ice@67cc918
pion/ice internally would spawn a new goroutine for each ICEConnectionState. This could cause things to get delivered out of order. We now use one routine and deliver in order.