fixeria has submitted this change. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/39631?usp=email ) Change subject: sctp_server: invalidate handler's Pid on 'EXIT' ...................................................................... sctp_server: invalidate handler's Pid on 'EXIT' An 'EXIT' signal indicates that a connection handling process is dead, so the respective pid shall not be referenced anymore. When this happens, the current implementation shuts the eNB association down gracefully and simply erases the respective client state from the clients dictionary. The problem with this approach is that we're bypassing the logic in client_del/2 and, as a result, never decrementing the ?S1GW_GAUGE_S1AP_ENB_NUM_SCTP_CONNECTIONS. When in sctp_recv/2 we receive a 'shutdown_comp' event and call client_del/2, the client state is already gone. Instead of erasing the client state on 'EXIT', invalidate handler's pid by setting it to undefined and keep the clients dictionary intact. Leave the task of dictionary manipulation up to client_{add,del} API. Change-Id: I21fedf8579baa54dc1e7ade348abffd7ee9b04c4 Related: SYS#7288 --- M src/sctp_server.erl 1 file changed, 9 insertions(+), 3 deletions(-) Approvals: Jenkins Builder: Verified pespin: Looks good to me, approved diff --git a/src/sctp_server.erl b/src/sctp_server.erl index 9b10bbe..0a42011 100644 --- a/src/sctp_server.erl +++ b/src/sctp_server.erl @@ -69,7 +69,7 @@ }). -record(client_state, {addr_port :: addr_port(), - pid :: pid() + pid :: pid() | undefined }). @@ -153,10 +153,12 @@ #server_state{sock = Sock, clients = Clients} = S0) -> ?LOG_DEBUG("Child process ~p terminated with reason ~p", [Pid, Reason]), case client_find(Pid, S0) of - {ok, {Aid, _Client}} -> + {ok, {Aid, C0}} -> %% gracefully close the eNB connection gen_sctp:eof(Sock, #sctp_assoc_change{assoc_id = Aid}), - S1 = S0#server_state{clients = dict:erase(Aid, Clients)}, + %% invalidate pid in the client's state + C1 = C0#client_state{pid = undefined}, + S1 = S0#server_state{clients = dict:store(Aid, C1, Clients)}, {noreply, S1}; error -> {noreply, S0} @@ -207,6 +209,9 @@ ?LOG_DEBUG("eNB connection (id=~p, ~p:~p) -> MME: ~p", [Aid, FromAddr, FromPort, Data]), s1gw_metrics:ctr_inc(?S1GW_CTR_S1AP_ENB_ALL_RX), case dict:find(Aid, Clients) of + {ok, #client_state{pid = undefined}} -> + ?LOG_NOTICE("eNB connection (id=~p, ~p:~p) -> MME data ignored (no handler)", + [Aid, FromAddr, FromPort]); {ok, #client_state{pid = Pid}} -> Handler:send_data(Pid, Data); error -> @@ -247,6 +252,7 @@ s1gw_metrics:gauge_dec(?S1GW_GAUGE_S1AP_ENB_NUM_SCTP_CONNECTIONS), S#server_state{clients = dict:erase(Aid, Clients)}; error -> + ?LOG_ERROR("eNB connection (id=~p) is not known to us?!?", [Aid]), S end. -- To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/39631?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: erlang/osmo-s1gw Gerrit-Branch: master Gerrit-Change-Id: I21fedf8579baa54dc1e7ade348abffd7ee9b04c4 Gerrit-Change-Number: 39631 Gerrit-PatchSet: 2 Gerrit-Owner: fixeria <vyanitskiy@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy@sysmocom.de> Gerrit-Reviewer: laforge <laforge@osmocom.org> Gerrit-Reviewer: pespin <pespin@sysmocom.de>