[libnice] Assert due to wrong transition

[Lorenzo Miniero]

Hi,

I added some comments on this Phabricator link a few days ago:

https://phabricator.freedesktop.org/T120

but I suspect it's unmonitored, so I thought I'd write here as well.
Apologies if you got the same info more than once.

Basically, some Janus users recently switched to libnice master and
encountered a crash originated by a wrong state transition. Digging in the
changes since 0.1.13, we found out about the g_assert that can get libnice
to crash if the ICE state transition is considered to be invalid.

>From the feedback we got on our forums, one of the crashes definitely
occurred while trying to switch from GATHERING to CONNECTED, which does
indeed seem not to be permitted in your state machine. A colleague had a
look at the code, and found what may be a possibile state violation in
conncheck.c:

https://github.com/libnice/libnice/blob/master/agent/conncheck.c#L3413

Apparently, the only condition avoiding a transition to CONNECTED is a
state different from READY, which includes GATHERING too. Neither of us are
experts in the libnice internals, though, so not sure if this is a red
herring.

Does that sound like a good place to look into? If not, what may be the
issue? Can it be that this unallowed transition is facilitated by our usage
of trickle ICE? A quick "hack" in this specific instance might be simply
"allowing" the GATHERING to CONNECTED transition
in agent_signal_component_state_change, but 1) there might be other
transitions we're not aware of, and 2) not sure if this would just hide
what might be a lingering problem that could come back to bite later.

Looking forward to your thoughts, thanks in advance,
Lorenzo
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/nice/attachments/20180305/b341ca3f/attachment.html>