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]),
--
To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/42293?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: erlang/osmo-s1gw
Gerrit-Branch: master
Gerrit-Change-Id: I8d71ad8300feefb0aecbf690a825a2b4e9f1102c
Gerrit-Change-Number: 42293
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <[email protected]>
