fixeria has uploaded this change for review.

View Change

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 change 42293. To unsubscribe, or for help writing mail filters, visit settings.