fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/42293?usp=email )
Change subject: pfcp_peer: replace watchdog process with a timer ......................................................................
pfcp_peer: replace watchdog process with a timer
The watchdog process is spawned bare (no link/monitor), so if it crashes its failure goes unnoticed, and if pfcp_peer restarts the orphaned watchdog may fire a stale cast at the new process.
The watchdog process exists solely to send a delayed message to pfcp_peer on timeout, and to be cancelled (by receiving heartbeat_response) when the response arrives. That's exactly what erlang:start_timer/3 does natively - no separate process needed.
Change-Id: I8d71ad8300feefb0aecbf690a825a2b4e9f1102c --- M src/pfcp_peer.erl 1 file changed, 10 insertions(+), 17 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/erlang/osmo-s1gw refs/changes/93/42293/1
diff --git a/src/pfcp_peer.erl b/src/pfcp_peer.erl index e1b3aa1..9910f5f 100644 --- a/src/pfcp_peer.erl +++ b/src/pfcp_peer.erl @@ -88,7 +88,7 @@
-record(heartbeat_state, {from :: undefined | gen_statem:from(), seq_nr :: pfcp_seq_nr(), - pid :: pid() + tref :: reference() }).
-record(peer_state, {seid :: pfcp_seid(), @@ -323,11 +323,12 @@
%% Heartbeat Req (timeout) handle_event(_State, - cast, heartbeat_request_watchdog, + info, {timeout, TRef, heartbeat_request_watchdog}, #peer_state{heartbeat = HB} = S) -> case HB of #heartbeat_state{from = From, - seq_nr = SeqNr} -> + seq_nr = SeqNr, + tref = TRef} -> ?LOG_NOTICE("Heartbeat Request (SeqNr=~p) timeout", [SeqNr]), s1gw_metrics:ctr_inc(?S1GW_CTR_PFCP_HEARTBEAT_REQ_TIMEOUT), if @@ -337,7 +338,7 @@ ok end, {keep_state, S#peer_state{heartbeat = undefined}}; - undefined -> + _ -> {keep_state, S} end;
@@ -550,25 +551,17 @@ case send_pdu({heartbeat_request, ReqIEs}, S0) of {ok, S1} -> s1gw_metrics:ctr_inc(?S1GW_CTR_PFCP_HEARTBEAT_REQ_TX), - Pid = spawn(fun heartbeat_request_watchdog/0), + TRef = erlang:start_timer(?PFCP_HEARTBEAT_REQ_TIMEOUT, self(), + heartbeat_request_watchdog), HB = #heartbeat_state{from = From, seq_nr = SeqNr, - pid = Pid}, + tref = TRef}, {ok, S1#peer_state{heartbeat = HB}}; Result -> Result end.
-heartbeat_request_watchdog() -> - receive - heartbeat_response -> - ok - after - ?PFCP_HEARTBEAT_REQ_TIMEOUT -> - gen_statem:cast(?MODULE, ?FUNCTION_NAME) - end. -
recv_heartbeat_response(#pfcp{type = heartbeat_response, seq_no = SeqNr, @@ -580,14 +573,14 @@ case HB of #heartbeat_state{from = From, seq_nr = SeqNr, - pid = Pid} -> + tref = TRef} -> + erlang:cancel_timer(TRef), if From =/= undefined -> gen_statem:reply(From, ok); true -> ok end, - Pid ! heartbeat_response, {ok, S#peer_state{heartbeat = undefined}}; _ -> ?LOG_NOTICE("Heartbeat Response (SeqNr=~p) was not expected", [SeqNr]),