From f6bb3dc325e686375b1dee283bd91c3068b682a1 Mon Sep 17 00:00:00 2001 From: Siri Hansen Date: Wed, 26 Feb 2014 14:36:55 +0100 Subject: Change spawn/1 + monitor/2 to spawn_monitor/1 to avoid deadlock crashdump_viewer:progress_pmap did sometimes hang since is spawned a process, then monitored it and waited for a specific DOWN message. When the process' work was very fast, it would exit before the call to monitor and the DOWN message would contain reason 'noproc' instead of the expected {pmap_done,Result}. By doing spawn_monitor/1 instead, the monitor is always set before the process exits so the deadlock is avoided. --- lib/observer/src/crashdump_viewer.erl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/observer/src/crashdump_viewer.erl b/lib/observer/src/crashdump_viewer.erl index a17efbccb0..0467e808e3 100644 --- a/lib/observer/src/crashdump_viewer.erl +++ b/lib/observer/src/crashdump_viewer.erl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 2003-2013. All Rights Reserved. +%% Copyright Ericsson AB 2003-2014. All Rights Reserved. %% %% The contents of this file are subject to the Erlang Public License, %% Version 1.1, (the "License"); you may not use this file except in @@ -2559,11 +2559,11 @@ progress_pmap(Report,File,Fun,List) -> {L1,L2} = if length(L)>=NPerProc -> lists:split(NPerProc,L); true -> {L,[]} % last chunk end, - P = spawn( + {P,_Ref} = + spawn_monitor( fun() -> progress_map(Collector,ReportInterval,File,Fun,L1) end), - erlang:monitor(process,P), {L2,[P|Ps]} end, {List,[]}, -- cgit v1.2.3 From 6a5b206e984ed28d257c6ab518b3ecbe5c6033d7 Mon Sep 17 00:00:00 2001 From: Siri Hansen Date: Mon, 17 Mar 2014 15:48:17 +0100 Subject: Fix crash in crashdump_viewer when node has multiple creations A node to which we have references to multiple instances (creations) will have an information line in the crashdump like this: Creation: 1 2 ... This would earlier crash because crashdump_viewer would try to do list_to_integer on the value after "Creation: ". This is now corrected. This correction also helps the case when the emulator is debug compiled, since the line could then be Creation: 1 (refc=1) --- lib/observer/src/cdv_dist_cb.erl | 12 +++++++++--- lib/observer/src/crashdump_viewer.erl | 8 +++++++- lib/observer/src/observer_lib.erl | 2 ++ lib/observer/test/crashdump_viewer_SUITE.erl | 14 +++++++++++++- 4 files changed, 31 insertions(+), 5 deletions(-) (limited to 'lib') diff --git a/lib/observer/src/cdv_dist_cb.erl b/lib/observer/src/cdv_dist_cb.erl index 3860324d6f..f7e6c9aded 100644 --- a/lib/observer/src/cdv_dist_cb.erl +++ b/lib/observer/src/cdv_dist_cb.erl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 2013. All Rights Reserved. +%% Copyright Ericsson AB 2013-2014. All Rights Reserved. %% %% The contents of this file are subject to the Erlang Public License, %% Version 1.1, (the "License"); you may not use this file except in @@ -22,7 +22,8 @@ get_info/1, get_detail_cols/1, get_details/1, - detail_pages/0]). + detail_pages/0, + format/1]). -include_lib("wx/include/wx.hrl"). -include("crashdump_viewer.hrl"). @@ -75,6 +76,11 @@ init_gen_page(Parent, Info) -> Fields = info_fields(), cdv_info_wx:start_link(Parent,{Fields,Info,[]}). +format({creations,Creations}) -> + string:join([integer_to_list(C) || C <- Creations],","); +format(D) -> + D. + %%%----------------------------------------------------------------- %%% Internal info_fields() -> @@ -83,7 +89,7 @@ info_fields() -> {"Type", conn_type}, {"Channel", channel}, {"Controller", {click,controller}}, - {"Creation", creation}, + {"Creation", {{format,fun format/1},creation}}, {"Extra Info", error}]}, {scroll_boxes, [{"Remote Links",1,{click,remote_links}}, diff --git a/lib/observer/src/crashdump_viewer.erl b/lib/observer/src/crashdump_viewer.erl index 0467e808e3..a08d27d070 100644 --- a/lib/observer/src/crashdump_viewer.erl +++ b/lib/observer/src/crashdump_viewer.erl @@ -1576,7 +1576,13 @@ get_nodeinfo(Fd,Nod) -> "Controller" -> get_nodeinfo(Fd,Nod#nod{controller=val(Fd)}); "Creation" -> - get_nodeinfo(Fd,Nod#nod{creation=list_to_integer(val(Fd))}); + %% Throwing away elements like "(refc=1)", which might be + %% printed from a debug compiled emulator. + Creations = lists:flatmap(fun(C) -> try [list_to_integer(C)] + catch error:badarg -> [] + end + end, string:tokens(val(Fd)," ")), + get_nodeinfo(Fd,Nod#nod{creation={creations,Creations}}); "Remote link" -> Procs = val(Fd), % e.g. "<0.31.0> <4322.54.0>" {Local,Remote} = split(Procs), diff --git a/lib/observer/src/observer_lib.erl b/lib/observer/src/observer_lib.erl index cedaf7d2b8..34c7b127ff 100644 --- a/lib/observer/src/observer_lib.erl +++ b/lib/observer/src/observer_lib.erl @@ -249,6 +249,8 @@ to_str({func, {F,A}}) when is_atom(F), is_integer(A) -> lists:concat([F, "/", A]); to_str({func, {F,'_'}}) when is_atom(F) -> atom_to_list(F); +to_str({{format,Fun},Value}) when is_function(Fun) -> + Fun(Value); to_str({A, B}) when is_atom(A), is_atom(B) -> lists:concat([A, ":", B]); to_str({M,F,A}) when is_atom(M), is_atom(F), is_integer(A) -> diff --git a/lib/observer/test/crashdump_viewer_SUITE.erl b/lib/observer/test/crashdump_viewer_SUITE.erl index 7a582436b4..91f349b6c1 100644 --- a/lib/observer/test/crashdump_viewer_SUITE.erl +++ b/lib/observer/test/crashdump_viewer_SUITE.erl @@ -428,8 +428,10 @@ do_create_dumps(DataDir,Rel) -> end. -%% Create a dump which has two visible nodes, one hidden and one +%% Create a dump which has three visible nodes, one hidden and one %% not connected node, and with monitors and links between nodes. +%% One of the visible nodes is stopped and started again in order to +%% get multiple creations. full_dist_dump(DataDir,Rel) -> Opt = rel_opt(Rel), Pz = "-pz \"" ++ filename:dirname(code:which(?MODULE)) ++ "\"", @@ -450,6 +452,15 @@ full_dist_dump(DataDir,Rel) -> get_response(P4), get_response(P1), + %% start, stop and start a node in order to get multiple 'creations' + {ok,N5} = ?t:start_node(n5,peer,Opt ++ PzOpt), + P51 = rpc:call(N5,?helper_mod,remote_proc,[P1,Creator]), + get_response(P51), + ?t:stop_node(N5), + {ok,N5} = ?t:start_node(n5,peer,Opt ++ PzOpt), + P52 = rpc:call(N5,?helper_mod,remote_proc,[P1,Creator]), + get_response(P52), + {aaaaaaaa,N1} ! {hello,from,other,node}, % distribution message ?t:stop_node(N3), @@ -458,6 +469,7 @@ full_dist_dump(DataDir,Rel) -> ?t:stop_node(N2), ?t:stop_node(N4), + ?t:stop_node(N5), CD. get_response(P) -> -- cgit v1.2.3 From f51d3ce81c1314d06b76975d9d06bf2b22925a89 Mon Sep 17 00:00:00 2001 From: Siri Hansen Date: Tue, 18 Mar 2014 09:09:20 +0100 Subject: Improve crashdump_viewer_SUITE:start_stop test case Instead of setting a timer and expecting all processes to be terminated, set a monitor and wait for them to terminate. --- lib/observer/test/crashdump_viewer_SUITE.erl | 68 +++++++++++++++++++++------- 1 file changed, 52 insertions(+), 16 deletions(-) (limited to 'lib') diff --git a/lib/observer/test/crashdump_viewer_SUITE.erl b/lib/observer/test/crashdump_viewer_SUITE.erl index 91f349b6c1..f6e7d18f65 100644 --- a/lib/observer/test/crashdump_viewer_SUITE.erl +++ b/lib/observer/test/crashdump_viewer_SUITE.erl @@ -120,27 +120,63 @@ start_stop(Config) when is_list(Config) -> Dump = hd(?config(dumps,Config)), timer:sleep(1000), - ProcsBefore = length(processes()), + ProcsBefore = processes(), + NumProcsBefore = length(ProcsBefore), ok = crashdump_viewer:start(Dump), - true = is_pid(whereis(crashdump_viewer_server)), - true = is_pid(whereis(cdv_wx)), - true = is_pid(whereis(cdv_proc_cb)), - true = is_pid(whereis(cdv_port_cb)), - true = is_pid(whereis(cdv_ets_cb)), - true = is_pid(whereis(cdv_timer_cb)), - true = is_pid(whereis(cdv_fun_cb)), - true = is_pid(whereis(cdv_atom_cb)), - true = is_pid(whereis(cdv_dist_cb)), - true = is_pid(whereis(cdv_mod_cb)), + ExpectedRegistered = [crashdump_viewer_server, + cdv_wx, + cdv_proc_cb, + cdv_proc_cb__holder, + cdv_port_cb, + cdv_port_cb__holder, + cdv_ets_cb, + cdv_ets_cb__holder, + cdv_timer_cb, + cdv_timer_cb__holder, + cdv_fun_cb, + cdv_fun_cb__holder, + cdv_atom_cb, + cdv_atom_cb__holder, + cdv_dist_cb, + cdv_dist_cb__holder, + cdv_mod_cb, + cdv_mod_cb__holder], + Regs=[begin + P=whereis(N), + {P,N,erlang:monitor(process,P)} + end || N <- ExpectedRegistered], + ct:log("CDV procs: ~n~p~n",[Regs]), + [true=is_pid(P) || {P,_,_} <- Regs], timer:sleep(5000), % give some time to live ok = crashdump_viewer:stop(), - timer:sleep(1000), % give some time to stop - undefined = whereis(crashdump_viewer_server), - undefined = whereis(cdv_wx), - ProcsAfter=length(processes()), - ProcsAfter=ProcsBefore, + recv_downs(Regs), + timer:sleep(2000), + ProcsAfter = processes(), + NumProcsAfter = length(ProcsAfter), + if NumProcsAfter=/=NumProcsBefore -> + ct:log("Before but not after:~n~p~n", + [[{P,process_info(P)} || P <- ProcsBefore -- ProcsAfter]]), + ct:log("After but not before:~n~p~n", + [[{P,process_info(P)} || P <- ProcsAfter -- ProcsBefore]]), + ct:fail("leaking processes"); + true -> + ok + end, ok. +recv_downs([]) -> + ok; +recv_downs(Regs) -> + receive + {'DOWN',Ref,process,_Pid,_} -> + ct:log("Got 'DOWN' for process ~n~p~n",[_Pid]), + recv_downs(lists:keydelete(Ref,3,Regs)) + after 30000 -> + ct:log("Timeout waiting for down:~n~p~n", + [[{Reg,process_info(P)} || {P,_,_}=Reg <- Regs]]), + ct:log("Message queue:~n~p~n",[process_info(self(),messages)]) + end. + %% Try to load nonexisting file non_existing(Config) when is_list(Config) -> ExpectedReason = "non-existing-file is not an Erlang crash dump\n", -- cgit v1.2.3