From 7d7271121b4d2e786bf6fb41508166efb96e5842 Mon Sep 17 00:00:00 2001
From: Anders Svensson <anders@erlang.org>
Date: Wed, 18 May 2016 11:34:48 +0200
Subject: Don't restart transport processes after transport removal

A replacement accepting transport could be started after the service
process received a shutdown message from diameter_config, if a
connection was accepted before the transport process in question was
terminated. The replacement lived on until the service needed to restart
it.
---
 lib/diameter/src/base/diameter_service.erl | 47 +++++++++++++++++++++---------
 1 file changed, 33 insertions(+), 14 deletions(-)

(limited to 'lib/diameter/src')

diff --git a/lib/diameter/src/base/diameter_service.erl b/lib/diameter/src/base/diameter_service.erl
index b1b4c7e050..ccf68f4d93 100644
--- a/lib/diameter/src/base/diameter_service.erl
+++ b/lib/diameter/src/base/diameter_service.erl
@@ -136,7 +136,7 @@
          state = ?WD_INITIAL :: match(wd_state()),
          started = diameter_lib:now(),%% at process start
          peer = false :: match(boolean() | pid())}).
-                      %% true at accepted, pid() at okay/reopen
+                      %% true at accepted/remove, pid() at okay/reopen
 
 %% Record representing a Peer State Machine processes implemented by
 %% diameter_peer_fsm.
@@ -676,25 +676,34 @@ mod_state(Alias, ModS) ->
 %% remove_transport
 shutdown(Refs, #state{watchdogT = WatchdogT})
   when is_list(Refs) ->
-    ets:foldl(fun(P,ok) -> st(P, Refs), ok end, ok, WatchdogT);
+    ets:insert(WatchdogT, ets:foldl(fun(R,A) -> st(R, Refs, A) end,
+                                    [],
+                                    WatchdogT));
 
 %% application/service shutdown
 shutdown(Reason, #state{watchdogT = WatchdogT})
   when Reason == application;
        Reason == service ->
-    diameter_lib:wait(ets:foldl(fun(P,A) -> st(P, Reason, A) end,
+    diameter_lib:wait(ets:foldl(fun(P,A) -> ss(P, Reason, A) end,
                                 [],
                                 WatchdogT)).
 
-%% st/2
+%% st/3
 
-st(#watchdog{ref = Ref, pid = Pid}, Refs) ->
-    lists:member(Ref, Refs)
-        andalso (Pid ! {shutdown, self(), transport}).  %% 'DOWN' cleans up
+%% Mark replacement as started so that a subsequent accept doesn't
+%% result in a new process that isn't terminated.
+st(#watchdog{ref = Ref, pid = Pid, peer = P} = Rec, Refs, Acc) ->
+    case lists:member(Ref, Refs) of
+        true ->
+            Pid ! {shutdown, self(), transport},  %% 'DOWN' cleans up
+            [Rec#watchdog{peer = true} || P == false] ++ Acc;
+        false ->
+            Acc
+    end.
 
-%% st/3
+%% ss/3
 
-st(#watchdog{pid = Pid}, Reason, Acc) ->
+ss(#watchdog{pid = Pid}, Reason, Acc) ->
     MRef = monitor(process, Pid),
     Pid ! {shutdown, self(), Reason},
     [MRef | Acc].
@@ -974,11 +983,22 @@ ms(_, Svc) ->
 %% ---------------------------------------------------------------------------
 
 accepted(Pid, _TPid, #state{watchdogT = WatchdogT} = S) ->
-    #watchdog{ref = Ref, type = accept = T, peer = false, options = Opts}
+    #watchdog{type = accept = T, peer = P}
         = Wd
         = fetch(WatchdogT, Pid),
-    ets:insert(WatchdogT, Wd#watchdog{peer = true}),%% mark replacement started
-    start(Ref, T, Opts, S).                         %% start new watchdog
+    if not P ->
+            #watchdog{ref = Ref, options = Opts} = Wd,
+            %% Mark replacement started, and start new watchdog.
+            ets:insert(WatchdogT, Wd#watchdog{peer = true}),
+            start(Ref, T, Opts, S);
+       P ->
+            %% Transport removal in progress: true has been set in
+            %% shutdown/2, and the transport will die as a
+            %% consequence.
+            ok
+    end.
+
+%% fetch/2
 
 fetch(Tid, Key) ->
     [T] = ets:lookup(Tid, Key),
@@ -1317,8 +1337,7 @@ start_tc(Tc, T, _) ->
 tc_timeout({Ref, _Type, _Opts} = T, #state{service_name = SvcName} = S) ->
     tc(diameter_config:have_transport(SvcName, Ref), T, S).
 
-tc(true, {Ref, Type, Opts}, #state{service_name = SvcName}
-                            = S) ->
+tc(true, {Ref, Type, Opts}, #state{service_name = SvcName} = S) ->
     send_event(SvcName, {reconnect, Ref, Opts}),
     start(Ref, Type, Opts, S);
 tc(false = No, _, _) ->  %% removed
-- 
cgit v1.2.3