Ensure dbg tracer exits when dbg is stopped

Previously the tracer might receive an 'EXIT' message in the trace
receive loop and treat it as garbage.

Test uses a custom dbg process handler to reliably recreate the
race condition, i.e. the message order {trace, ..}, {'EXIT, ..}.

2 files changed, 64 insertions, 32 deletions

diff --git a/lib/runtime_tools/src/dbg.erl b/lib/runtime_tools/src/dbg.erl
index 186563ab74..c2de57d40b 100644
--- a/lib/runtime_tools/src/dbg.erl
+++ b/lib/runtime_tools/src/dbg.erl
@@ -778,50 +778,50 @@ tracer_init(Handler, HandlerData) ->
     tracer_loop(Handler, HandlerData).
 
 tracer_loop(Handler, Hdata) ->
-    receive
-	Msg ->
-	    %% Don't match in receive to avoid giving EXIT message higher
-	    %% priority than the trace messages.
-	    case Msg of
-		{'EXIT',_Pid,_Reason} ->
-		    ok;
-		Trace ->
-		    NewData = recv_all_traces(Trace, Handler, Hdata),
-		    tracer_loop(Handler, NewData)
-	    end
+    {State, Suspended, Traces} =  recv_all_traces(),
+    NewHdata = handle_traces(Suspended, Traces, Handler, Hdata),
+    case State of
+	done ->
+	    exit(normal);
+	loop ->
+	    tracer_loop(Handler, NewHdata)
     end.
-    
-recv_all_traces(Trace, Handler, Hdata) ->
-    Suspended = suspend(Trace, []),
-    recv_all_traces(Suspended, Handler, Hdata, [Trace]).
 
-recv_all_traces(Suspended0, Handler, Hdata, Traces) ->
+recv_all_traces() ->
+    recv_all_traces([], [], infinity).
+
+recv_all_traces(Suspended0, Traces, Timeout) ->
     receive
 	Trace when is_tuple(Trace), element(1, Trace) == trace ->
 	    Suspended = suspend(Trace, Suspended0),
-	    recv_all_traces(Suspended, Handler, Hdata, [Trace|Traces]);
+	    recv_all_traces(Suspended, [Trace|Traces], 0);
 	Trace when is_tuple(Trace), element(1, Trace) == trace_ts ->
 	    Suspended = suspend(Trace, Suspended0),
-	    recv_all_traces(Suspended, Handler, Hdata, [Trace|Traces]);
+	    recv_all_traces(Suspended, [Trace|Traces], 0);
 	Trace when is_tuple(Trace), element(1, Trace) == seq_trace ->
 	    Suspended = suspend(Trace, Suspended0),
-	    recv_all_traces(Suspended, Handler, Hdata, [Trace|Traces]);
+	    recv_all_traces(Suspended, [Trace|Traces], 0);
 	Trace when is_tuple(Trace), element(1, Trace) == drop ->
 	    Suspended = suspend(Trace, Suspended0),
-	    recv_all_traces(Suspended, Handler, Hdata, [Trace|Traces]);
+	    recv_all_traces(Suspended, [Trace|Traces], 0);
+	{'EXIT', _Pid, _Reason} ->
+	    {done, Suspended0, Traces};
 	Other ->
 	    %%% Is this really a good idea?
 	    io:format(user,"** tracer received garbage: ~p~n", [Other]),
-	    recv_all_traces(Suspended0, Handler, Hdata, Traces)
-    after 0 ->
-	    case catch invoke_handler(Traces, Handler, Hdata) of
-		{'EXIT',Reason} -> 
-		    resume(Suspended0),
-		    exit({trace_handler_crashed,Reason});
-		NewHdata ->
-		    resume(Suspended0),
-		    NewHdata
-	    end
+	    recv_all_traces(Suspended0, Traces, Timeout)
+    after Timeout ->
+	    {loop, Suspended0, Traces}
+    end.
+
+handle_traces(Suspended, Traces, Handler, Hdata) ->
+    case catch invoke_handler(Traces, Handler, Hdata) of
+	{'EXIT',Reason} -> 
+	    resume(Suspended),
+	    exit({trace_handler_crashed,Reason});
+	NewHdata ->
+	    resume(Suspended),
+	    NewHdata
     end.
 
 invoke_handler([Tr|Traces], Handler, Hdata0) ->
diff --git a/lib/runtime_tools/test/dbg_SUITE.erl b/lib/runtime_tools/test/dbg_SUITE.erl
index dfae52ed1d..0bcbd67d05 100644
--- a/lib/runtime_tools/test/dbg_SUITE.erl
+++ b/lib/runtime_tools/test/dbg_SUITE.erl
@@ -25,7 +25,7 @@
 	 ip_port/1, file_port/1, file_port2/1, file_port_schedfix/1,
 	 ip_port_busy/1, wrap_port/1, wrap_port_time/1,
 	 with_seq_trace/1, dead_suspend/1, local_trace/1,
-	 saved_patterns/1]).
+	 saved_patterns/1, tracer_exit_on_stop/1]).
 -export([init_per_testcase/2, end_per_testcase/2]).
 -export([tracee1/1, tracee2/1]).
 -export([dummy/0, exported/1]).
@@ -47,7 +47,7 @@ all() ->
     [big, tiny, simple, message, distributed, ip_port,
      file_port, file_port2, file_port_schedfix, ip_port_busy,
      wrap_port, wrap_port_time, with_seq_trace, dead_suspend,
-     local_trace, saved_patterns].
+     local_trace, saved_patterns, tracer_exit_on_stop].
 
 groups() -> 
     [].
@@ -742,6 +742,38 @@ run_dead_suspend() ->
 dummy() ->
     ok.
 
+%% Test that a tracer process does not ignore an exit signal message when it has
+%% received (but not handled) trace messages
+tracer_exit_on_stop(_) ->
+    %% Tracer blocks waiting for fun to complete so that the trace message and
+    %% the exit signal message from the dbg process are in its message queue.
+    Fun = fun() ->
+	    ?MODULE:dummy(),
+	    Ref = erlang:trace_delivered(self()),
+	    receive {trace_delivered, _, Ref} -> stop() end
+	end,
+    {ok, _} = dbg:tracer(process, {fun spawn_once_handler/2, {self(), Fun}}),
+    {ok, Tracer} = dbg:get_tracer(),
+    MRef = monitor(process, Tracer),
+    {ok, _} = dbg:p(self(), [call]),
+    {ok, _} = dbg:p(new, [call]),
+    {ok, _} = dbg:tp(?MODULE, dummy, []),
+    ?MODULE:dummy(),
+    receive {'DOWN', MRef, _, _, normal} -> ok end,
+    [{trace,_,call,{?MODULE, dummy,[]}},
+     {trace,_,call,{?MODULE, dummy,[]}}] = flush(),
+    ok.
+
+spawn_once_handler(Event, {Pid, done} = State) ->
+    Pid ! Event,
+    State;
+spawn_once_handler(Event, {Pid, Fun}) ->
+    {_, Ref} = spawn_monitor(Fun),
+    receive
+	{'DOWN', Ref, _, _, _} ->
+	    Pid ! Event,
+	    {Pid, done}
+    end.
 
 %%
 %% Support functions