Timeout pairs after not receiving data for long enough (#40)

README.md

@@ -17,7 +17,7 @@ Implements:
 * role conflict resolution
 * supports host, prflx, srflx and relay candidates
 * transaction pacing
-* keepalives on valid and selected pairs
+* keepalives (both incoming and outgoing) on valid and selected pairs
 * mDNS client
 
 ## Limitations

lib/ex_ice/ice_agent.ex

@@ -341,7 +341,19 @@ defmodule ExICE.ICEAgent do
 
  @impl true
  def handle_info(:ta_timeout, state) do
- ice_agent = ExICE.Priv.ICEAgent.handle_timeout(state.ice_agent)
+ ice_agent = ExICE.Priv.ICEAgent.handle_ta_timeout(state.ice_agent)
+ {:noreply, %{state | ice_agent: ice_agent}}
+ end
+
+ @impl true
+ def handle_info(:eoc_timeout, state) do
+ ice_agent = ExICE.Priv.ICEAgent.handle_eoc_timeout(state.ice_agent)
+ {:noreply, %{state | ice_agent: ice_agent}}
+ end
+
+ @impl true
+ def handle_info(:pair_timeout, state) do
+ ice_agent = ExICE.Priv.ICEAgent.handle_pair_timeout(state.ice_agent)
  {:noreply, %{state | ice_agent: ice_agent}}
  end
 

lib/ex_ice/priv/candidate_pair.ex

@@ -20,7 +20,8 @@ defmodule ExICE.Priv.CandidatePair do
  valid?: boolean,
  succeeded_pair_id: integer() | nil,
  discovered_pair_id: integer() | nil,
- keepalive_timer: reference() | nil
+ keepalive_timer: reference() | nil,
+ last_seen: integer()
  }
 
  @enforce_keys [:id, :local_cand_id, :remote_cand_id, :priority]
@@ -32,7 +33,10 @@ defmodule ExICE.Priv.CandidatePair do
  valid?: false,
  succeeded_pair_id: nil,
  discovered_pair_id: nil,
- keepalive_timer: nil
+ keepalive_timer: nil,
+ # Time when this pair has received some data
+ # or sent conn check.
+ last_seen: nil
  ]
 
  @doc false
@@ -47,7 +51,8 @@ defmodule ExICE.Priv.CandidatePair do
  remote_cand_id: remote_cand.id,
  priority: priority,
  state: state,
- valid?: opts[:valid?] || false
+ valid?: opts[:valid?] || false,
+ last_seen: opts[:last_seen]
  }
  end
 

lib/ex_ice/priv/conn_check_handler/controlled.ex

@@ -15,20 +15,15 @@ defmodule ExICE.Priv.ConnCheckHandler.Controlled do
  nil ->
  Logger.debug("Adding new candidate pair: #{inspect(pair)}")
  checklist = Map.put(ice_agent.checklist, pair.id, pair)
- %ICEAgent{ice_agent | checklist: checklist}
-
- %CandidatePair{} = pair
- when ice_agent.selected_pair != nil and
- pair.discovered_pair_id == ice_agent.selected_pair.id ->
- # to be honest this might also be a retransmission
- Logger.debug("Keepalive on selected pair: #{pair.discovered_pair_id}")
- ice_agent
-
- %CandidatePair{} ->
- # keepalive/retransmission?
- ice_agent
+ ice_agent = %ICEAgent{ice_agent | checklist: checklist}
+ ICEAgent.send_binding_success_response(ice_agent, pair, msg)
+
+ %CandidatePair{} = checklist_pair ->
+ checklist_pair = %CandidatePair{checklist_pair | last_seen: pair.last_seen}
+ checklist = Map.put(ice_agent.checklist, checklist_pair.id, checklist_pair)
+ ice_agent = %ICEAgent{ice_agent | checklist: checklist}
+ ICEAgent.send_binding_success_response(ice_agent, checklist_pair, msg)
  end
- |> ICEAgent.send_binding_success_response(pair, msg)
  end
 
  @impl true
@@ -43,32 +38,42 @@ defmodule ExICE.Priv.ConnCheckHandler.Controlled do
 
  pair = %CandidatePair{pair | nominate?: true}
  checklist = Map.put(ice_agent.checklist, pair.id, pair)
- %ICEAgent{ice_agent | checklist: checklist}
 
- %CandidatePair{} = pair
- when ice_agent.selected_pair != nil and
- pair.discovered_pair_id == ice_agent.selected_pair.id ->
- Logger.debug("Keepalive on selected pair: #{pair.id}")
- ice_agent
-
- %CandidatePair{} = pair ->
- if pair.state == :succeeded do
- Logger.debug("Nomination request on pair: #{pair.id}.")
- update_nominated_flag(ice_agent, pair.discovered_pair_id, true)
+ ice_agent = %ICEAgent{ice_agent | checklist: checklist}
+ ICEAgent.send_binding_success_response(ice_agent, pair, msg)
+
+ %CandidatePair{} = checklist_pair ->
+ if checklist_pair.state == :succeeded do
+ discovered_pair = Map.fetch!(ice_agent.checklist, checklist_pair.discovered_pair_id)
+ discovered_pair = %CandidatePair{discovered_pair | last_seen: pair.last_seen}
+ ice_agent = put_in(ice_agent.checklist[discovered_pair.id], discovered_pair)
+
+ if ice_agent.selected_pair_id == nil do
+ Logger.debug("Nomination request on pair: #{discovered_pair.id}.")
+ update_nominated_flag(ice_agent, discovered_pair.id, true)
+ else
+ ice_agent
+ end
+ |> ICEAgent.send_binding_success_response(discovered_pair, msg)
  else
  # TODO should we check if this pair is not in failed?
  Logger.debug("""
  Nomination request on pair that hasn't been verified yet.
  We will nominate pair once conn check passes.
- Pair: #{inspect(pair.id)}
+ Pair: #{inspect(checklist_pair.id)}
  """)
 
- pair = %CandidatePair{pair | nominate?: true}
- checklist = Map.put(ice_agent.checklist, pair.id, pair)
- %ICEAgent{ice_agent | checklist: checklist}
+ checklist_pair = %CandidatePair{
+ checklist_pair
+ | nominate?: true,
+ last_seen: pair.last_seen
+ }
+
+ checklist = Map.put(ice_agent.checklist, checklist_pair.id, checklist_pair)
+ ice_agent = %ICEAgent{ice_agent | checklist: checklist}
+ ICEAgent.send_binding_success_response(ice_agent, checklist_pair, msg)
  end
  end
- |> ICEAgent.send_binding_success_response(pair, msg)
  end
 
  @impl true
@@ -85,20 +90,26 @@ defmodule ExICE.Priv.ConnCheckHandler.Controlled do
  ice_agent = %ICEAgent{ice_agent | checklist: checklist}
 
  cond do
- ice_agent.selected_pair == nil ->
+ ice_agent.selected_pair_id == nil ->
  Logger.debug("Selecting pair: #{pair_id}")
- %ICEAgent{ice_agent | selected_pair: pair}
+ %ICEAgent{ice_agent | selected_pair_id: pair.id}
 
- ice_agent.selected_pair != nil and pair.priority >= ice_agent.selected_pair.priority ->
- Logger.debug("""
- Selecting new pair with higher priority. \
- New pair: #{pair_id}, old pair: #{ice_agent.selected_pair.id}.\
- """)
+ ice_agent.selected_pair_id != nil and pair.id != ice_agent.selected_pair_id ->
+ selected_pair = Map.fetch!(ice_agent.checklist, ice_agent.selected_pair_id)
 
- %ICEAgent{ice_agent | selected_pair: pair}
+ if pair.priority >= selected_pair.priority do
+ Logger.debug("""
+ Selecting new pair with higher priority. \
+ New pair: #{pair_id}, old pair: #{ice_agent.selected_pair_id}.\
+ """)
+
+ %ICEAgent{ice_agent | selected_pair_id: pair.id}
+ else
+ ice_agent
+ end
 
  true ->
- Logger.debug("Not selecting a new pair as it has lower priority")
+ Logger.debug("Not selecting a new pair as it has lower priority or has the same id")
  ice_agent
  end
  end

lib/ex_ice/priv/conn_check_handler/controlling.ex

@@ -26,20 +26,15 @@ defmodule ExICE.Priv.ConnCheckHandler.Controlling do
  nil ->
  Logger.debug("Adding new candidate pair: #{inspect(pair)}")
  checklist = Map.put(ice_agent.checklist, pair.id, pair)
- %ICEAgent{ice_agent | checklist: checklist}
-
- %CandidatePair{} = pair
- when ice_agent.selected_pair != nil and
- pair.discovered_pair_id == ice_agent.selected_pair.id ->
- # to be honest this might also be a retransmission
- Logger.debug("Keepalive on selected pair: #{pair.discovered_pair_id}")
- ice_agent
-
- %CandidatePair{} ->
- # keepalive/retransmission?
- ice_agent
+ ice_agent = %ICEAgent{ice_agent | checklist: checklist}
+ ICEAgent.send_binding_success_response(ice_agent, pair, msg)
+
+ %CandidatePair{} = checklist_pair ->
+ checklist_pair = %CandidatePair{checklist_pair | last_seen: pair.last_seen}
+ checklist = Map.put(ice_agent.checklist, checklist_pair.id, checklist_pair)
+ ice_agent = %ICEAgent{ice_agent | checklist: checklist}
+ ICEAgent.send_binding_success_response(ice_agent, checklist_pair, msg)
  end
- |> ICEAgent.send_binding_success_response(pair, msg)
  end
 
  @impl true
@@ -61,6 +56,6 @@ defmodule ExICE.Priv.ConnCheckHandler.Controlling do
  Logger.warning("Nomination succeeded but checklist hasn't finished.")
  end
 
- %ICEAgent{ice_agent | nominating?: {false, nil}, selected_pair: pair}
+ %ICEAgent{ice_agent | nominating?: {false, nil}, selected_pair_id: pair.id}
  end
 end