jitsi/ice4j

Ice4j goes to 'TERMINATED' state on failure

bbaldino opened this issue · 2 comments

bbaldino commented

During some testing I noticed ice4j transitioning to TERMINATED state, which is supposed to represent success, in a failure case (ICE did not connect successfully). I ran into this when looking at re-enabling ICE connections for health checks in the new bridge code. The case is not exactly valid--it involves 2 ice agents, both in single port mode (using the same port) and both marked as controlling--but still we should not transition to TERMINATED here. Attached bridge logs with ice4j logging on FINEST.

ice4j_running_to_terminated_bug.log

mstyura commented

Just a small investigation, because I've touched related code in my PRs.
There are two paces which calls terminate(IceProcessingState.TERMINATED);

  1. From Agent.free. There is no Free ICE agent (logged at the top of free()) line in logs you've attached and there is a state check before calling terminate(IceProcessingState.TERMINATED);, so the issue you're encountered caused not by call from free().
  2. And from Agent.terminationRunnable, which I've introduced in #150 as a replacement to blocked Thread. So, it is likely that moving to IceProcessingState.TERMINATED caused by this line. The runnable is scheduled or called synchronously (depending on delay config) from scheduleTermination method, which is only called form checkListStatesUpdated. Unfortunately there are no logged lines from checkListStatesUpdated found in log you've attached it is hard to tell if checkListStatesUpdated caused problem.

What is strange with checkListStatesUpdated is there is a return from a method, if atLeastOneListSucceeded == fasle, which has default value false and only set to true here, which should go with a "CheckList of stream " + stream.getName() + " is COMPLETED" line in log, but it is absent in what you've attached.

It might be the case that I've looked at source code of different (current master) ice4j version from what you've actually tested.

Could you please give move details about issue: ice4j version and if it is possible to reproduce with existing bridge somehow?

bbaldino commented

Hey @mstyura, thanks for looking at this! It was just something I ran into that I wanted to make sure we tracked, but you reminded me of an important point: the ice4j version I'm using is old; it's version 2.0.0-20180713.194548-10.

  • 👍1