Skip to content

Commit

Permalink
Clear selected pair on ICE failed
Browse files Browse the repository at this point in the history
When ICE times out and fails, all candidates are deleted.

That means all the candidates are closed and their underlying
conns are clsoed.

But, the selected pair could still be valid. On a subsequenct
`Write`, ICE transport conn will get the selected pair and
write to the pair. As the pair is still valid, write will
flow through to the local candidate `writeTo`.

But, as all candidates and their underlying conns are closed,
`Write` will return a `io.ErrClosedPipe` error.

There are cases where it is not ignored and causes a broken
pipe after an ICERestart.

When the `Write` error propagates back to sctp/association,
the writeLoop is exited.

So, sending data channel traffic after a successful ICERestart
still fails as the SCTP association errored out and write loop exited.

I have copied the changes that are done when ICERestart happens
to when ICE state is set to failed (except for gathering state
and resetting ufrag/pwd). In my testing, it is working well,
i. e. can continue data channel after ICE Restart whereas
previously it was failing every time. But, I am not sure of all
the implications of this change.
  • Loading branch information
boks1971 committed Sep 2, 2023
1 parent 949c2e3 commit d58cb33
Showing 1 changed file with 4 additions and 0 deletions.
4 changes: 4 additions & 0 deletions agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,10 @@ func (a *Agent) updateConnectionState(newState ConnectionState) {
if a.connectionState != newState {
// Connection has gone to failed, release all gathered candidates
if newState == ConnectionStateFailed {
a.removeUfragFromMux()
a.checklist = make([]*CandidatePair, 0)
a.pendingBindingRequests = make([]bindingRequest, 0)
a.setSelectedPair(nil)

Check warning on line 500 in agent.go

View check run for this annotation

Codecov / codecov/patch

agent.go#L497-L500

Added lines #L497 - L500 were not covered by tests
a.deleteAllCandidates()
}

Expand Down

0 comments on commit d58cb33

Please sign in to comment.