[cover] Allow reconnection if node has been disconnected or down

OTP-10523

Earlier, if the connection to a remote cover node was lost, all cover
data was lost and the cover_server on the remote node would die. This
would cause problems if there were cover compiled modules that would
still be executed since they would attempt to write to the no longer
existing ets tables belonging to the cover_server.

This commit changes this behavior so that the cover_server on the
remote node will survive connection loss and continue collecting cover
data. If the connection is re-established then the main node will sync
with the remote node again and cover data will not be lost (unless the
node was down).

1 files changed, 100 insertions, 39 deletions

diff --git a/lib/tools/src/cover.erl b/lib/tools/src/cover.erl
index 2154a21ed6..10f14b0a49 100644
--- a/lib/tools/src/cover.erl
+++ b/lib/tools/src/cover.erl
@@ -26,12 +26,17 @@
 %% ARCHITECTURE
 %% The coverage tool consists of one process on each node involved in
 %% coverage analysis. The process is registered as 'cover_server'
-%% (?SERVER).  All cover_servers in the distributed system are linked
-%% together.  The cover_server on the 'main' node is in charge, and it
-%% traps exits so it can detect nodedown or process crashes on the
-%% remote nodes. This process is implemented by the functions
-%% init_main/1 and main_process_loop/1. The cover_server on the remote
-%% nodes are implemented by the functions init_remote/2 and
+%% (?SERVER).  The cover_server on the 'main' node is in charge, and
+%% it monitors the cover_servers on all remote nodes. When it gets a
+%% 'DOWN' message for another cover_server, it marks the node as
+%% 'lost'. If a nodeup is received for a lost node the main node
+%% ensures that the cover compiled modules are loaded again. If the
+%% remote node was alive during the disconnected periode, cover data
+%% for this periode will also be included in the analysis.
+%%
+%% The cover_server process on the main node is implemented by the
+%% functions init_main/1 and main_process_loop/1. The cover_server on
+%% the remote nodes are implemented by the functions init_remote/2 and
 %% remote_process_loop/1.
 %%
 %% TABLES
@@ -90,7 +95,8 @@
 -record(main_state, {compiled=[],           % [{Module,File}]
 		     imported=[],           % [{Module,File,ImportFile}]
 		     stopper,               % undefined | pid()
-		     nodes=[]}).            % [Node]
+		     nodes=[],              % [Node]
+		     lost_nodes=[]}).       % [Node]
 
 -record(remote_state, {compiled=[],         % [{Module,File}]
 		       main_node}).         % atom()
@@ -586,39 +592,14 @@ init_main(Starter) ->
     ets:new(?BINARY_TABLE, [set, named_table]),
     ets:new(?COLLECTION_TABLE, [set, public, named_table]),
     ets:new(?COLLECTION_CLAUSE_TABLE, [set, public, named_table]),
+    net_kernel:monitor_nodes(true),
     Starter ! {?SERVER,started},
     main_process_loop(#main_state{}).
 
 main_process_loop(State) ->
     receive
 	{From, {start_nodes,Nodes}} ->
-	    ThisNode = node(),
-	    StartedNodes = 
-		lists:foldl(
-		  fun(Node,Acc) ->
-			  case rpc:call(Node,cover,remote_start,[ThisNode]) of
-			      {ok,_RPid} ->
-				  erlang:monitor(process,{?SERVER,Node}),
-				  [Node|Acc];
-			      Error ->
-				  io:format("Could not start cover on ~w: ~p\n",
-					    [Node,Error]),
-				  Acc
-			  end
-		  end,
-		  [],
-		  Nodes),
-
-	    %% In case some of the compiled modules have been unloaded they 
-	    %% should not be loaded on the new node.
-	    {_LoadedModules,Compiled} = 
-		get_compiled_still_loaded(State#main_state.nodes,
-					  State#main_state.compiled),
-	    remote_load_compiled(StartedNodes,Compiled),
-	    
-	    State1 = 
-		State#main_state{nodes = State#main_state.nodes ++ StartedNodes,
-				 compiled = Compiled},
+	    {StartedNodes,State1} = do_start_nodes(Nodes, State),
 	    reply(From, {ok,StartedNodes}),
 	    main_process_loop(State1);
 
@@ -810,9 +791,24 @@ main_process_loop(State) ->
 	    main_process_loop(S);		    
 	
 	{'DOWN', _MRef, process, {?SERVER,Node}, _Info} ->
-	    %% A remote cover_server is down, remove node from list.
-	    Nodes1 = State#main_state.nodes--[Node],
-	    main_process_loop(State#main_state{nodes=Nodes1});
+	    %% A remote cover_server is down, mark as lost
+	    Nodes = State#main_state.nodes--[Node],
+	    Lost = [Node|State#main_state.lost_nodes],
+	    main_process_loop(State#main_state{nodes=Nodes,lost_nodes=Lost});
+
+	{nodeup,Node} ->
+	    State1 =
+		case lists:member(Node,State#main_state.lost_nodes) of
+		    true ->
+			sync_compiled(Node,State);
+		    false ->
+			State
+	    end,
+	    main_process_loop(State1);
+
+	{nodedown,_} ->
+	    %% Will be taken care of when 'DOWN' message arrives
+	    main_process_loop(State);
 	
 	{From, get_main_node} ->
 	    reply(From, node()),
@@ -874,8 +870,13 @@ remote_process_loop(State) ->
             unregister(?SERVER),
 	    ok; % not replying since 'DOWN' message will be received anyway
 
+	{remote,get_compiled} ->
+	    remote_reply(State#remote_state.main_node,
+			 State#remote_state.compiled),
+	    remote_process_loop(State);
+
 	{From, get_main_node} ->
-	    reply(From, State#remote_state.main_node),
+	    remote_reply(From, State#remote_state.main_node),
 	    remote_process_loop(State);
 
 	get_status ->
@@ -987,6 +988,36 @@ unload([]) ->
 
 %%%--Handling of remote nodes--------------------------------------------
 
+do_start_nodes(Nodes, State) ->
+    ThisNode = node(),
+    StartedNodes =
+	lists:foldl(
+	  fun(Node,Acc) ->
+		  case rpc:call(Node,cover,remote_start,[ThisNode]) of
+		      {ok,_RPid} ->
+			  erlang:monitor(process,{?SERVER,Node}),
+			  [Node|Acc];
+		      Error ->
+			  io:format("Could not start cover on ~w: ~p\n",
+				    [Node,Error]),
+			  Acc
+		  end
+	  end,
+	  [],
+	  Nodes),
+
+    %% In case some of the compiled modules have been unloaded they
+    %% should not be loaded on the new node.
+    {_LoadedModules,Compiled} =
+	get_compiled_still_loaded(State#main_state.nodes,
+				  State#main_state.compiled),
+    remote_load_compiled(StartedNodes,Compiled),
+
+    State1 =
+	State#main_state{nodes = State#main_state.nodes ++ StartedNodes,
+			 compiled = Compiled},
+    {StartedNodes, State1}.
+
 %% start the cover_server on a remote node
 remote_start(MainNode) ->
     case whereis(?SERVER) of
@@ -1010,6 +1041,30 @@ remote_start(MainNode) ->
 	    {error,{already_started,Pid}}
     end.
 
+%% If a lost node comes back, ensure that main and remote node has the
+%% same cover compiled modules. Note that no action is taken if the
+%% same {Mod,File} eksists on both, i.e. code change is not handled!
+sync_compiled(Node,State) ->
+    #main_state{compiled=Compiled0,nodes=Nodes,lost_nodes=Lost}=State,
+    State1 =
+	case remote_call(Node,{remote,get_compiled}) of
+	    {error,node_dead} ->
+		{_,S} = do_start_nodes([Node],State),
+		S;
+	    {error,_} ->
+		State;
+	    RemoteCompiled ->
+		{_,Compiled} =  get_compiled_still_loaded(Nodes,Compiled0),
+		Unload = [UM || {UM,_}=U <- RemoteCompiled,
+			       false == lists:member(U,Compiled)],
+		remote_unload([Node],Unload),
+		Load = [L || L <- Compiled,
+			     false == lists:member(L,RemoteCompiled)],
+		remote_load_compiled([Node],Load),
+		State#main_state{compiled=Compiled, nodes=[Node|Nodes]}
+	end,
+    State1#main_state{lost_nodes=Lost--[Node]}.
+
 %% Load a set of cover compiled modules on remote nodes,
 %% We do it ?MAX_MODS modules at a time so that we don't
 %% run out of memory on the cover_server node. 
@@ -2279,7 +2334,13 @@ do_reset2([]) ->
 do_clear(Module) ->
     ets:match_delete(?COVER_CLAUSE_TABLE, {Module,'_'}),
     ets:match_delete(?COVER_TABLE, {#bump{module=Module},'_'}),
-    ets:match_delete(?COLLECTION_TABLE, {#bump{module=Module},'_'}).
+    case lists:member(?COLLECTION_TABLE, ets:all()) of
+	true ->
+	    %% We're on the main node
+	    ets:match_delete(?COLLECTION_TABLE, {#bump{module=Module},'_'});
+	false ->
+	    ok
+    end.
 
 not_loaded(Module, unloaded, State) ->
     do_clear(Module),