From cbbc2588a1f9acea502b0fde0fcac507b02ba1ea Mon Sep 17 00:00:00 2001 From: Anders Svensson <anders@erlang.org> Date: Fri, 15 Dec 2017 13:07:24 +0100 Subject: Fix diameter_reg:subscribe/2 remove notification Commit fae8ca0c broke notification by removing table elements before matching for them, causing diameter_tcp/sctp listening processes to live on after diameter:remove_transport/2. Commit 58091992 added diameter_reg:subscribe/2, and commit 5ca5fb71 started using it for listener exit. --- lib/diameter/src/base/diameter_reg.erl | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/lib/diameter/src/base/diameter_reg.erl b/lib/diameter/src/base/diameter_reg.erl index 5b7cfab31a..c1762a07e3 100644 --- a/lib/diameter/src/base/diameter_reg.erl +++ b/lib/diameter/src/base/diameter_reg.erl @@ -246,8 +246,11 @@ handle_call({add, Uniq, Key}, {Pid, _}, S) -> handle_call({remove, Key}, {Pid, _}, S) -> Rec = {Key, Pid}, - ets:delete_object(?TABLE, Rec), - {reply, true, notify(remove, Rec, S)}; + {reply, true, try + notify(remove, Rec, S) + after + ets:delete_object(?TABLE, Rec) + end}; handle_call({wait, Pat}, {Pid, _} = From, S) -> NS = add_monitor(Pid, S), @@ -370,10 +373,12 @@ send({_,_} = From, add, Rec) -> down(Pid, #state{monitors = Ps} = S) -> Recs = match('_', Pid), - ets:match_delete(?TABLE, {'_', Pid}), - lists:foldl(fun(R,NS) -> notify(remove, R, NS) end, - flush(Pid, S#state{monitors = sets:del_element(Pid, Ps)}), - Recs). + Acc0 = flush(Pid, S#state{monitors = sets:del_element(Pid, Ps)}), + try + lists:foldl(fun(R,NS) -> notify(remove, R, NS) end, Acc0, Recs) + after + ets:match_delete(?TABLE, {'_', Pid}) + end. %% flush/3 -- cgit v1.2.3 From d73df2f7850184a8e1f44fdc82217935c25e867b Mon Sep 17 00:00:00 2001 From: Anders Svensson <anders@erlang.org> Date: Thu, 1 Feb 2018 11:11:55 +0100 Subject: Fix handling of SUSPECT connections at service termination A peer connection in watchdog state SUSPECT is represented by a peer table entry in diameter_service, but not by a request table entry in diameter_peer, so diameter_service:terminate/2 could result in failures like this: exception error: no match of right hand side value [] in function diameter_traffic:peer_down/1 (base/diameter_traffic.erl, line 141) in call from lists:foldl/3 (lists.erl, line 1263) in call from ets:do_foldl/4 (ets.erl, line 611) in call from ets:foldl/3 (ets.erl, line 600) in call from diameter_service:terminate/2 (base/diameter_service.erl, line 557) in call from gen_server:try_terminate/3 (gen_server.erl, line 648) in call from gen_server:terminate/10 (gen_server.erl, line 833) in call from gen_server:handle_msg/6 (gen_server.erl, line 679) --- lib/diameter/src/base/diameter_service.erl | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/lib/diameter/src/base/diameter_service.erl b/lib/diameter/src/base/diameter_service.erl index 31dd92f878..55ebcafcf9 100644 --- a/lib/diameter/src/base/diameter_service.erl +++ b/lib/diameter/src/base/diameter_service.erl @@ -554,15 +554,25 @@ terminate(Reason, #state{service_name = Name, local = {PeerT, _, _}} = S) -> %% wait for watchdog state changes to take care of if. That this %% takes place after deleting the state entry ensures that the %% resulting failover by request processes accomplishes nothing. - ets:foldl(fun(#peer{pid = TPid}, _) -> - diameter_traffic:peer_down(TPid) - end, - ok, - PeerT), + ets:foldl(fun peer_down/2, ok, PeerT), shutdown == Reason %% application shutdown andalso shutdown(application, S). +%% peer_down/1 +%% +%% Entries with watchdog state SUSPECT are already down: ignore the +%% expected failure. This assumes the current implementation, but +%% double the number of lookups (in the typical case) could be the +%% greater evil if there are many peer connections. + +peer_down(#peer{pid = TPid}, _) -> + try + diameter_traffic:peer_down(TPid) + catch + error: {badmatch, []} -> ok + end. + %% --------------------------------------------------------------------------- %% # code_change/3 %% --------------------------------------------------------------------------- -- cgit v1.2.3