Skip to content

Commit 994acb5

Browse files
mickel8mickel8
authored
Don't clear entire state after moving to the failed or completed state (#69)
1 parent 125a04c commit 994acb5

File tree

9 files changed

+1145
-220
lines changed

9 files changed

+1145
-220
lines changed

‎lib/ex_ice/priv/candidate_base.ex

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ defmodule ExICE.Priv.CandidateBase do
1313
transport: :udp,
1414
transport_module: module(),
1515
socket: :inet.socket() | nil,
16-
type: Candidate.type()
16+
type: Candidate.type(),
17+
closed?: boolean()
1718
}
1819

1920
@derive {Inspect, except: [:socket]}
@@ -27,7 +28,7 @@ defmodule ExICE.Priv.CandidateBase do
2728
:transport_module,
2829
:type
2930
]
30-
defstruct @enforce_keys ++ [:base_address, :base_port, :socket]
31+
defstruct @enforce_keys ++ [:base_address, :base_port, :socket, closed?: false]
3132

3233
@spec new(Candidate.type(), Keyword.t()) :: t()
3334
def new(type, config) do

‎lib/ex_ice/priv/checklist.ex

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,10 @@ defmodule ExICE.Priv.Checklist do
1717
@spec get_pair_for_nomination(t()) :: CandidatePair.t() | nil
1818
def get_pair_for_nomination(checklist) do
1919
checklist
20-
|> Enum.filter(fn {_id, pair} -> pair.valid? end)
20+
# pair might have been marked as failed if the associated
21+
# local candidate has been closed
22+
|> Stream.filter(fn {_id, pair} -> pair.state == :succeeded end)
23+
|> Stream.filter(fn {_id, pair} -> pair.valid? end)
2124
|> Enum.max_by(fn {_id, pair} -> pair.priority end, fn -> {nil, nil} end)
2225
|> elem(1)
2326
end
@@ -26,6 +29,9 @@ defmodule ExICE.Priv.Checklist do
2629
def get_valid_pair(checklist) do
2730
checklist
2831
|> Stream.map(fn {_id, pair} -> pair end)
32+
# pair might have been marked as failed if the associated
33+
# local candidate has been closed
34+
|> Stream.filter(fn pair -> pair.state == :succeeded end)
2935
|> Stream.filter(fn pair -> pair.valid? end)
3036
|> Enum.sort_by(fn pair -> pair.priority end, :desc)
3137
|> Enum.at(0)
@@ -79,11 +85,16 @@ defmodule ExICE.Priv.Checklist do
7985
Map.new(waiting ++ in_flight_or_done)
8086
end
8187

82-
@spec prune(t(), Candidate.t()) :: t()
83-
def prune(checklist, local_cand) do
84-
checklist
85-
|> Enum.reject(fn {_pair_id, pair} -> pair.local_cand_id == local_cand.base.id end)
86-
|> Map.new()
88+
@spec close_candidate(t(), Candidate.t()) :: {[integer()], t()}
89+
def close_candidate(checklist, local_cand) do
90+
Enum.reduce(checklist, {[], checklist}, fn {pair_id, pair}, {failed_pair_ids, checklist} ->
91+
if pair.local_cand_id == local_cand.base.id and pair.state != :failed do
92+
checklist = Map.put(checklist, pair_id, %{pair | state: :failed})
93+
{[pair_id | failed_pair_ids], checklist}
94+
else
95+
{failed_pair_ids, checklist}
96+
end
97+
end)
8798
end
8899

89100
@spec timeout_pairs(t(), [integer()]) :: t()

‎lib/ex_ice/priv/conn_check_handler/controlled.ex

Lines changed: 50 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -12,30 +12,60 @@ defmodule ExICE.Priv.ConnCheckHandler.Controlled do
1212
def handle_conn_check_request(ice_agent, pair, msg, nil) do
1313
# TODO use triggered check queue
1414
case Checklist.find_pair(ice_agent.checklist, pair) do
15-
nil ->
15+
nil when ice_agent.state in [:completed, :failed] ->
16+
# This is to make sure we won't add a new pair with prflx candidate
17+
# after we have moved to the completed or failed state.
18+
# Alternatively, we could answer with error message.
19+
Logger.warning("""
20+
Received conn check request for non-existing pair in unexpected state: #{ice_agent.state}. Ignoring\
21+
""")
22+
23+
ice_agent
24+
25+
nil when ice_agent.state in [:new, :checking, :connected] ->
1626
Logger.debug("Adding new candidate pair: #{inspect(pair)}")
1727
checklist = Map.put(ice_agent.checklist, pair.id, pair)
1828
ice_agent = %ICEAgent{ice_agent | checklist: checklist}
1929
ICEAgent.send_binding_success_response(ice_agent, pair, msg)
2030

2131
%CandidatePair{} = checklist_pair ->
22-
checklist_pair =
23-
if checklist_pair.state == :failed do
24-
%CandidatePair{checklist_pair | state: :waiting, last_seen: pair.last_seen}
25-
else
26-
%CandidatePair{checklist_pair | last_seen: pair.last_seen}
27-
end
28-
29-
ice_agent = put_in(ice_agent.checklist[checklist_pair.id], checklist_pair)
30-
ICEAgent.send_binding_success_response(ice_agent, checklist_pair, msg)
32+
cond do
33+
checklist_pair.state == :failed and ice_agent.state in [:failed, :completed] ->
34+
# update last seen so we can observe that something is received but don't reply
35+
# as we are in the failed state
36+
checklist_pair = %CandidatePair{checklist_pair | last_seen: pair.last_seen}
37+
put_in(ice_agent.checklist[checklist_pair.id], checklist_pair)
38+
39+
checklist_pair.state == :failed ->
40+
checklist_pair = %CandidatePair{
41+
checklist_pair
42+
| state: :waiting,
43+
last_seen: pair.last_seen
44+
}
45+
46+
ice_agent = put_in(ice_agent.checklist[checklist_pair.id], checklist_pair)
47+
ICEAgent.send_binding_success_response(ice_agent, checklist_pair, msg)
48+
49+
true ->
50+
checklist_pair = %CandidatePair{checklist_pair | last_seen: pair.last_seen}
51+
ice_agent = put_in(ice_agent.checklist[checklist_pair.id], checklist_pair)
52+
ICEAgent.send_binding_success_response(ice_agent, checklist_pair, msg)
53+
end
3154
end
3255
end
3356

3457
@impl true
3558
def handle_conn_check_request(ice_agent, pair, msg, %UseCandidate{}) do
3659
# TODO use triggered check queue
3760
case Checklist.find_pair(ice_agent.checklist, pair) do
38-
nil ->
61+
nil when ice_agent.state in [:completed, :failed] ->
62+
Logger.warning("""
63+
Received conn check request for non-existing pair in unexpected state: #{ice_agent.state}. Ignoring\
64+
""")
65+
66+
ice_agent
67+
68+
nil when ice_agent.state in [:new, :checking, :connected] ->
3969
Logger.debug("""
4070
Adding new candidate pair that will be nominated after \
4171
successful conn check: #{inspect(pair.id)}\
@@ -47,20 +77,21 @@ defmodule ExICE.Priv.ConnCheckHandler.Controlled do
4777
ice_agent = %ICEAgent{ice_agent | checklist: checklist}
4878
ICEAgent.send_binding_success_response(ice_agent, pair, msg)
4979

50-
%CandidatePair{state: :succeeded} = checklist_pair ->
80+
%CandidatePair{state: :succeeded} = checklist_pair when ice_agent.state != :failed ->
5181
discovered_pair = Map.fetch!(ice_agent.checklist, checklist_pair.discovered_pair_id)
5282
discovered_pair = %CandidatePair{discovered_pair | last_seen: pair.last_seen}
5383
ice_agent = put_in(ice_agent.checklist[discovered_pair.id], discovered_pair)
5484

55-
if ice_agent.selected_pair_id == nil do
85+
if ice_agent.selected_pair_id != discovered_pair.id do
5686
Logger.debug("Nomination request on pair: #{discovered_pair.id}.")
5787
update_nominated_flag(ice_agent, discovered_pair.id, true)
5888
else
5989
ice_agent
6090
end
6191
|> ICEAgent.send_binding_success_response(discovered_pair, msg)
6292

63-
%CandidatePair{state: :failed} = checklist_pair ->
93+
%CandidatePair{state: :failed} = checklist_pair
94+
when ice_agent.state not in [:completed, :failed] ->
6495
Logger.debug("""
6596
Nomination request on failed pair. Re-scheduling pair for conn-check.
6697
We will nominate pair once conn check passes.
@@ -77,7 +108,7 @@ defmodule ExICE.Priv.ConnCheckHandler.Controlled do
77108
ice_agent = put_in(ice_agent.checklist[checklist_pair.id], checklist_pair)
78109
ICEAgent.send_binding_success_response(ice_agent, checklist_pair, msg)
79110

80-
%CandidatePair{} = checklist_pair ->
111+
%CandidatePair{} = checklist_pair when ice_agent.state not in [:completed, :failed] ->
81112
Logger.debug("""
82113
Nomination request on pair that hasn't been verified yet.
83114
We will nominate pair once conn check passes.
@@ -92,6 +123,10 @@ defmodule ExICE.Priv.ConnCheckHandler.Controlled do
92123

93124
ice_agent = put_in(ice_agent.checklist[checklist_pair.id], checklist_pair)
94125
ICEAgent.send_binding_success_response(ice_agent, checklist_pair, msg)
126+
127+
%CandidatePair{} = checklist_pair ->
128+
checklist_pair = %CandidatePair{checklist_pair | last_seen: pair.last_seen}
129+
put_in(ice_agent.checklist[checklist_pair.id], checklist_pair)
95130
end
96131
end
97132

‎lib/ex_ice/priv/conn_check_handler/controlling.ex

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,22 +27,42 @@ defmodule ExICE.Priv.ConnCheckHandler.Controlling do
2727
def handle_conn_check_request(ice_agent, pair, msg, nil) do
2828
# TODO use triggered check queue
2929
case Checklist.find_pair(ice_agent.checklist, pair) do
30-
nil ->
30+
nil when ice_agent.state in [:completed, :failed] ->
31+
Logger.warning("""
32+
Received conn check request for non-existing pair in unexpected state: #{ice_agent.state}. Ignoring\
33+
""")
34+
35+
ice_agent
36+
37+
nil when ice_agent.state in [:new, :checking, :connected] ->
3138
Logger.debug("Adding new candidate pair: #{inspect(pair)}")
3239
checklist = Map.put(ice_agent.checklist, pair.id, pair)
3340
ice_agent = %ICEAgent{ice_agent | checklist: checklist}
3441
ICEAgent.send_binding_success_response(ice_agent, pair, msg)
3542

3643
%CandidatePair{} = checklist_pair ->
37-
checklist_pair =
38-
if checklist_pair.state == :failed do
39-
%CandidatePair{checklist_pair | state: :waiting, last_seen: pair.last_seen}
40-
else
41-
%CandidatePair{checklist_pair | last_seen: pair.last_seen}
42-
end
43-
44-
ice_agent = put_in(ice_agent.checklist[checklist_pair.id], checklist_pair)
45-
ICEAgent.send_binding_success_response(ice_agent, checklist_pair, msg)
44+
cond do
45+
checklist_pair.state == :failed and ice_agent.state in [:failed, :completed] ->
46+
# update last seen so we can observe that something is received but don't reply
47+
# as we are in the failed state
48+
checklist_pair = %CandidatePair{checklist_pair | last_seen: pair.last_seen}
49+
put_in(ice_agent.checklist[checklist_pair.id], checklist_pair)
50+
51+
checklist_pair.state == :failed ->
52+
checklist_pair = %CandidatePair{
53+
checklist_pair
54+
| state: :waiting,
55+
last_seen: pair.last_seen
56+
}
57+
58+
ice_agent = put_in(ice_agent.checklist[checklist_pair.id], checklist_pair)
59+
ICEAgent.send_binding_success_response(ice_agent, checklist_pair, msg)
60+
61+
true ->
62+
checklist_pair = %CandidatePair{checklist_pair | last_seen: pair.last_seen}
63+
ice_agent = put_in(ice_agent.checklist[checklist_pair.id], checklist_pair)
64+
ICEAgent.send_binding_success_response(ice_agent, checklist_pair, msg)
65+
end
4666
end
4767
end
4868

0 commit comments

Comments
 (0)