Ensure capabilities exchange can't fail too early

In particular, not before the service process has a monitor on
the watchdog since the watchdog's exit reason is meaningful.

2 files changed, 43 insertions, 17 deletions

diff --git a/lib/diameter/src/base/diameter_service.erl b/lib/diameter/src/base/diameter_service.erl
index 0893956f97..3dfdcee2b2 100644
--- a/lib/diameter/src/base/diameter_service.erl
+++ b/lib/diameter/src/base/diameter_service.erl
@@ -629,10 +629,6 @@ insert(Tbl, Rec) ->
     ets:insert(Tbl, Rec),
     Rec.
 
-monitor(Pid) ->
-    erlang:monitor(process, Pid),
-    Pid.
-
 %% Using the process dictionary for the callback state was initially
 %% just a way to make what was horrendous trace (big state record and
 %% much else everywhere) somewhat more readable. There's not as much
@@ -814,10 +810,10 @@ start(Ref, Type, Opts, #state{peerT = PeerT,
                               service = Svc})
   when Type == connect;
        Type == accept ->
-    Pid = monitor(s(Type, Ref, {ConnT,
-                                Opts,
-                                SvcName,
-                                merge_service(Opts, Svc)})),
+    Pid = s(Type, Ref, {ConnT,
+                        Opts,
+                        SvcName,
+                        merge_service(Opts, Svc)}),
     insert(PeerT, #peer{pid = Pid,
                         type = Type,
                         ref = Ref,
@@ -830,7 +826,13 @@ start(Ref, Type, Opts, #state{peerT = PeerT,
 %% callbacks.
 
 s(Type, Ref, T) ->
-    diameter_watchdog:start({Type, Ref}, T).
+    case diameter_watchdog:start({Type, Ref}, T) of
+        {_MRef, Pid} ->
+            Pid;
+        Pid when is_pid(Pid) ->  %% from old code
+            erlang:monitor(process, Pid),
+            Pid
+    end.
 
 %% merge_service/2
 
diff --git a/lib/diameter/src/base/diameter_watchdog.erl b/lib/diameter/src/base/diameter_watchdog.erl
index 6dc53d9f31..fb22fd8275 100644
--- a/lib/diameter/src/base/diameter_watchdog.erl
+++ b/lib/diameter/src/base/diameter_watchdog.erl
@@ -59,10 +59,19 @@
          message_data}).   %% term passed into diameter_service with message
 
 %% start/2
+%%
+%% Start a monitor before the watchdog is allowed to proceed to ensure
+%% that a failed capabilities exchange produces the desired exit
+%% reason.
 
 start({_,_} = Type, T) ->
-    {ok, Pid} = diameter_watchdog_sup:start_child({Type, self(), T}),
-    Pid.
+    Ref = make_ref(),
+    {ok, Pid} = diameter_watchdog_sup:start_child({Ref, {Type, self(), T}}),
+    try
+        {erlang:monitor(process, Pid), Pid}
+    after
+        Pid ! Ref
+    end.
 
 start_link(T) ->
     {ok, _} = proc_lib:start_link(?MODULE,
@@ -80,14 +89,29 @@ init(T) ->
     proc_lib:init_ack({ok, self()}),
     gen_server:enter_loop(?MODULE, [], i(T)).
 
-i({T, Pid, {ConnT, Opts, SvcName, #diameter_service{applications = Apps,
-                                                    capabilities = Caps}
-                                  = Svc}}) ->
-    {M,S,U} = now(),
-    random:seed(M,S,U),
+i({Ref, {_, Pid, _} = T}) ->
+    MRef = erlang:monitor(process, Pid),
+    receive
+        Ref ->
+            make_state(T);
+        {'DOWN', MRef, process, _, _} = D ->
+            exit({shutdown, D})
+    end;
+
+i({_, Pid, _} = T) ->  %% from old code
+    erlang:monitor(process, Pid),
+    make_state(T).
+
+make_state({T, Pid, {ConnT,
+                     Opts,
+                     SvcName,
+                     #diameter_service{applications = Apps,
+                                       capabilities = Caps}
+                     = Svc}}) ->
+    random:seed(now()),
     putr(restart, {T, Opts, Svc}),  %% save seeing it in trace
     putr(dwr, dwr(Caps)),           %%
-    #watchdog{parent = monitor(Pid),
+    #watchdog{parent = Pid,
               transport = monitor(diameter_peer_fsm:start(T, Opts, Svc)),
               tw = proplists:get_value(watchdog_timer,
                                        Opts,