Merge remote branch 'origin/peppe/common_test/ct_conn_handling' into maint

* origin/peppe/common_test/ct_conn_handling:
  Add test suite
  Fix faulty connection handling

OTP-10126

7 files changed, 122 insertions, 117 deletions

diff --git a/lib/common_test/src/ct_config.erl b/lib/common_test/src/ct_config.erl
index 9bb9817001..5c80a299f8 100644
--- a/lib/common_test/src/ct_config.erl
+++ b/lib/common_test/src/ct_config.erl
@@ -46,7 +46,7 @@
 	 decrypt_config_file/2, decrypt_config_file/3,
 	 get_crypt_key_from_file/0, get_crypt_key_from_file/1]).
 
--export([get_ref_from_name/1, get_name_from_ref/1, get_key_from_name/1]).
+-export([get_key_from_name/1]).
 
 -export([check_config_files/1, add_default_callback/1, prepare_config_list/1]).
 
@@ -56,7 +56,7 @@
 
 -define(cryptfile, ".ct_config.crypt").
 
--record(ct_conf,{key,value,handler,config,ref,name='_UNDEF',default=false}).
+-record(ct_conf,{key,value,handler,config,name='_UNDEF',default=false}).
 
 start(Mode) ->
     case whereis(ct_config_server) of
@@ -275,7 +275,6 @@ store_config(Config, Callback, File) when is_list(Config) ->
 			 value=Val,
 			 handler=Callback,
 			 config=File,
-			 ref=ct_util:ct_make_ref(),
 			 default=false}) ||
 	{Key,Val} <- Config].
 
@@ -296,13 +295,11 @@ rewrite_config(Config, Callback, File) ->
 			   #ct_conf{key=Key,
 				    value=Value,
 				    handler=Callback,
-				    config=File,
-				    ref=ct_util:ct_make_ref()});
+				    config=File});
 	    RowsToUpdate ->
 		Inserter = fun(Row) ->
 				   ets:insert(?attr_table,
-					      Row#ct_conf{value=Value,
-							  ref=ct_util:ct_make_ref()})
+					      Row#ct_conf{value=Value})
 			   end,
 		lists:foreach(Inserter, RowsToUpdate)
 	end
@@ -314,7 +311,7 @@ set_config(Config,Default) ->
 
 set_config(Name,Config,Default) ->
     [ets:insert(?attr_table,
-		#ct_conf{key=Key,value=Val,ref=ct_util:ct_make_ref(),
+		#ct_conf{key=Key,value=Val,
 			 name=Name,default=Default}) ||
 	{Key,Val} <- Config].
 
@@ -559,26 +556,6 @@ encrypt_config_file(SrcFileName, EncryptFileName) ->
 	    encrypt_config_file(SrcFileName, EncryptFileName, {key,Key})
     end.
 
-get_ref_from_name(Name) ->
-    case ets:select(?attr_table,[{#ct_conf{name=Name,ref='$1',_='_'},
-				  [],
-				  ['$1']}]) of
-	[Ref] ->
-	    {ok,Ref};
-	_ ->
-	    {error,{no_such_name,Name}}
-    end.
-
-get_name_from_ref(Ref) ->
-    case ets:select(?attr_table,[{#ct_conf{name='$1',ref=Ref,_='_'},
-				  [],
-				  ['$1']}]) of
-	[Name] ->
-	    {ok,Name};
-	_ ->
-	    {error,{no_such_ref,Ref}}
-    end.
-
 get_key_from_name(Name) ->
     case ets:select(?attr_table,[{#ct_conf{name=Name,key='$1',_='_'},
 				  [],
diff --git a/lib/common_test/src/ct_ftp.erl b/lib/common_test/src/ct_ftp.erl
index 8790393b36..b91a521bd4 100644
--- a/lib/common_test/src/ct_ftp.erl
+++ b/lib/common_test/src/ct_ftp.erl
@@ -348,10 +348,10 @@ terminate(FtpPid,State) ->
 get_handle(Pid) when is_pid(Pid) ->
     {ok,Pid};
 get_handle(Name) ->
-    case ct_util:get_connections(Name,?MODULE) of
-	{ok,[{Pid,_}|_]} ->
+    case ct_util:get_connection(Name,?MODULE) of
+	{ok,{Pid,_}} ->
 	    {ok,Pid};
-	{ok,[]} ->
+	{error,no_registered_connection} ->
 	    open(Name);
 	Error ->
 	    Error
diff --git a/lib/common_test/src/ct_gen_conn.erl b/lib/common_test/src/ct_gen_conn.erl
index 2d4b1d1f52..a5b736136f 100644
--- a/lib/common_test/src/ct_gen_conn.erl
+++ b/lib/common_test/src/ct_gen_conn.erl
@@ -26,7 +26,7 @@
 
 -compile(export_all).
 
--export([start/4, stop/1]).
+-export([start/4, stop/1, get_conn_pid/1]).
 -export([call/2, call/3, return/2, do_within_time/2]).
 
 -ifdef(debug).
@@ -120,8 +120,16 @@ start(Name,Address,InitData,CallbackMod) ->
 %%%      Handle = handle()
 %%%
 %%% @doc Close the connection and stop the process managing it.
-stop(Pid) ->
-    call(Pid,stop,5000).
+stop(Handle) ->
+    call(Handle,stop,5000).
+
+%%%-----------------------------------------------------------------
+%%% @spec get_conn_pid(Handle) -> ok
+%%%      Handle = handle()
+%%%
+%%% @doc Return the connection pid associated with Handle
+get_conn_pid(Handle) ->
+    call(Handle,get_conn_pid).
 
 %%%-----------------------------------------------------------------
 %%% @spec log(Heading,Format,Args) -> ok
@@ -222,7 +230,8 @@ do_start(Opts) ->
     receive
 	{connected,Pid} ->
 	    erlang:demonitor(MRef, [flush]),
-	    ct_util:register_connection(Opts#gen_opts.name, Opts#gen_opts.address,
+	    ct_util:register_connection(Opts#gen_opts.name,
+					Opts#gen_opts.address,
 					Opts#gen_opts.callback, Pid),
 	    {ok,Pid};
 	{Error,Pid} ->
@@ -315,10 +324,12 @@ loop(Opts) ->
 			{ok, NewPid, NewState} ->
 			    link(NewPid),
 			    put(conn_pid,NewPid),
-			    loop(Opts#gen_opts{conn_pid=NewPid,cb_state=NewState});
+			    loop(Opts#gen_opts{conn_pid=NewPid,
+					       cb_state=NewState});
 			Error ->
 			    ct_util:unregister_connection(self()),
-			    log("Reconnect failed. Giving up!","Reason: ~p\n",
+			    log("Reconnect failed. Giving up!",
+				"Reason: ~p\n",
 				[Error])
 		    end;
 		false ->
@@ -338,7 +349,8 @@ loop(Opts) ->
 					       Opts#gen_opts.cb_state),
 	    return(From,ok),
 	    ok;
-	{{retry,{Error,_Name,CPid,_Msg}}, From} when CPid == Opts#gen_opts.conn_pid ->
+	{{retry,{Error,_Name,CPid,_Msg}}, From} when 
+	      CPid == Opts#gen_opts.conn_pid ->
 	    %% only retry if failure is because of a reconnection
 	    Return = case Error of
 			 {error,_} -> Error;
@@ -347,12 +359,16 @@ loop(Opts) ->
 	    return(From, Return),
 	    loop(Opts);
 	{{retry,{_Error,_Name,_CPid,Msg}}, From} ->
-	    log("Rerunning command","Connection reestablished. Rerunning command...",[]),
+	    log("Rerunning command","Connection reestablished. "
+		"Rerunning command...",[]),
 	    {Return,NewState} =
 		(Opts#gen_opts.callback):handle_msg(Msg,Opts#gen_opts.cb_state),
 	    return(From, Return),
 	    loop(Opts#gen_opts{cb_state=NewState});
-	{Msg,From={Pid,_Ref}} when is_pid(Pid), Opts#gen_opts.old==true ->
+	{get_conn_pid, From} ->
+	    return(From, Opts#gen_opts.conn_pid),
+	    loop(Opts);
+	{Msg, From={Pid,_Ref}} when is_pid(Pid), Opts#gen_opts.old==true ->
 	    {Return,NewState} =
 		(Opts#gen_opts.callback):handle_msg(Msg,Opts#gen_opts.cb_state),
 	    return(From, Return),
@@ -372,7 +388,8 @@ loop(Opts) ->
 		    return(From,Reply)
 	    end;
 	Msg when Opts#gen_opts.forward==true ->
-	    case (Opts#gen_opts.callback):handle_msg(Msg,Opts#gen_opts.cb_state) of
+	    case (Opts#gen_opts.callback):handle_msg(Msg,
+						     Opts#gen_opts.cb_state) of
 		{noreply,NewState} ->
 		    loop(Opts#gen_opts{cb_state=NewState});
 		{stop,NewState} ->
diff --git a/lib/common_test/src/ct_netconfc.erl b/lib/common_test/src/ct_netconfc.erl
index 28586c310e..e094ee877a 100644
--- a/lib/common_test/src/ct_netconfc.erl
+++ b/lib/common_test/src/ct_netconfc.erl
@@ -1164,13 +1164,11 @@ call(Client, Msg, Timeout, WaitStop) ->
 get_handle(Client) when is_pid(Client) ->
     {ok,Client};
 get_handle(Client) ->
-    case ct_util:get_connections(Client, ?MODULE) of
-	{ok,[{Pid,_}]} ->
+    case ct_util:get_connection(Client, ?MODULE) of
+	{ok,{Pid,_}} ->
 	    {ok,Pid};
-	{ok,[]} ->
+	{error,no_registered_connection} ->
 	    {error,{no_connection_found,Client}};
-	{ok,Conns} ->
-	    {error,{multiple_connections_found,Client,Conns}};
 	Error ->
 	    Error
     end.
diff --git a/lib/common_test/src/ct_ssh.erl b/lib/common_test/src/ct_ssh.erl
index c6ea27b10e..1adc79d358 100644
--- a/lib/common_test/src/ct_ssh.erl
+++ b/lib/common_test/src/ct_ssh.erl
@@ -1328,10 +1328,10 @@ do_recv_response(SSH, Chn, Data, End, Timeout) ->
 get_handle(SSH) when is_pid(SSH) ->
     {ok,SSH};
 get_handle(SSH) ->
-    case ct_util:get_connections(SSH, ?MODULE) of
-	{ok,[{Pid,_}]} ->
+    case ct_util:get_connection(SSH, ?MODULE) of
+	{ok,{Pid,_}} ->
 	    {ok,Pid};
-	{ok,[]} ->
+	{error,no_registered_connection} ->
 	    connect(SSH);
 	Error ->
 	    Error
diff --git a/lib/common_test/src/ct_telnet.erl b/lib/common_test/src/ct_telnet.erl
index 4755d939e0..4092d33bc0 100644
--- a/lib/common_test/src/ct_telnet.erl
+++ b/lib/common_test/src/ct_telnet.erl
@@ -183,7 +183,8 @@ open(KeyOrName,ConnType,TargetMod,Extra) ->
 			end;
 		    Bool -> Bool
 		end,
-	    log(heading(open,{KeyOrName,ConnType}),"Opening connection to: ~p",[Addr1]),
+	    log(heading(open,{KeyOrName,ConnType}),
+		"Opening connection to: ~p",[Addr1]),
 	    ct_gen_conn:start(KeyOrName,full_addr(Addr1,ConnType),
 			      {TargetMod,KeepAlive,Extra},?MODULE)
     end.
@@ -591,9 +592,9 @@ terminate(TelnPid,State) ->
 get_handle(Pid) when is_pid(Pid) ->
     {ok,Pid};
 get_handle({Name,Type}) when Type==telnet;Type==ts1;Type==ts2 ->
-    case ct_util:get_connections(Name,?MODULE) of
-	{ok,Conns} when Conns /= [] ->
-	    case get_handle(Type,Conns) of
+    case ct_util:get_connection(Name,?MODULE) of
+	{ok,Conn} ->
+	    case get_handle(Type,Conn) of
 		{ok,Pid} -> 
 		    {ok,Pid};
 		_Error ->
@@ -608,19 +609,15 @@ get_handle({Name,Type}) when Type==telnet;Type==ts1;Type==ts2 ->
 			    Error
 		    end
 	    end;
-	{ok,[]} ->
-	    {error,already_closed};
 	Error ->
 	    Error
     end;
 get_handle(Name) ->
     get_handle({Name,telnet}).
 
-get_handle(Type,[{Pid,{_,_,Type}}|_]) ->
+get_handle(Type,{Pid,{_,_,Type}}) ->
     {ok,Pid};
-get_handle(Type,[_H|T]) ->
-    get_handle(Type,T);
-get_handle(Type,[]) ->
+get_handle(Type,_) ->
     {error,{no_such_connection,Type}}.
 
 full_addr({Ip,Port},Type) ->
diff --git a/lib/common_test/src/ct_util.erl b/lib/common_test/src/ct_util.erl
index b77845eb5b..68e76c2396 100644
--- a/lib/common_test/src/ct_util.erl
+++ b/lib/common_test/src/ct_util.erl
@@ -25,13 +25,13 @@
 %%%
 -module(ct_util).
 
--export([start/0,start/1,start/2,start/3,
-	 stop/1,update_last_run_index/0]).
+-export([start/0, start/1, start/2, start/3,
+	 stop/1, update_last_run_index/0]).
 
--export([register_connection/4,unregister_connection/1,
-	 does_connection_exist/3,get_key_from_name/1]).
+-export([register_connection/4, unregister_connection/1,
+	 does_connection_exist/3, get_key_from_name/1]).
 
--export([close_connections/0]).
+-export([get_connections/1, close_connections/0]).
 
 -export([save_suite_data/3, save_suite_data/2,
 	 save_suite_data_async/3, save_suite_data_async/2,
@@ -56,11 +56,11 @@
 
 -export([listenv/1]).
 
--export([get_target_name/1, get_connections/2]).
+-export([get_target_name/1, get_connection/2]).
 
 -export([is_test_dir/1, get_testdir/2]).
 
--export([kill_attached/2, get_attached/1, ct_make_ref/0]).
+-export([kill_attached/2, get_attached/1]).
 
 -export([warn_duplicates/1]).
 
@@ -417,7 +417,8 @@ loop(Mode,TestData,StartDir) ->
 					 ?MAX_IMPORTANCE,
 					 "CT Error Notification",
 					 "Connection process died: "
-					 "Pid: ~w, Address: ~p, Callback: ~w\n"
+					 "Pid: ~w, Address: ~p, "
+					 "Callback: ~w\n"
 					 "Reason: ~p\n\n",
 					 [Pid,A,CB,Reason]),
 		    catch CB:close(Pid),
@@ -426,8 +427,8 @@ loop(Mode,TestData,StartDir) ->
 		    loop(Mode,TestData,StartDir);
 		_ ->
 		    %% Let process crash in case of error, this shouldn't happen!
-		    io:format("\n\nct_util_server got EXIT from ~w: ~p\n\n",
-			      [Pid,Reason]),
+		    io:format("\n\nct_util_server got EXIT "
+			      "from ~w: ~p\n\n", [Pid,Reason]),
 		    file:set_cwd(StartDir),
 		    exit(Reason)
 	    end
@@ -457,10 +458,13 @@ get_key_from_name(Name)->
 %%% table, and ct_util will close all registered connections when the
 %%% test is finished by calling <code>Callback:close/1</code>.</p>
 register_connection(TargetName,Address,Callback,Handle) ->
+    %% If TargetName is a registered alias for a config
+    %% variable, use it as reference for the connection,
+    %% otherwise use the Handle value.
     TargetRef = 
-	case ct_config:get_ref_from_name(TargetName) of
-	    {ok,Ref} ->
-		Ref;
+	case ct_config:get_key_from_name(TargetName) of
+	    {ok,_Key} ->
+		TargetName;
 	    _ ->
 		%% no config name associated with connection,
 		%% use handle for identification instead
@@ -496,10 +500,10 @@ unregister_connection(Handle) ->
 %%%
 %%% @doc Check if a connection already exists.
 does_connection_exist(TargetName,Address,Callback) ->
-    case ct_config:get_ref_from_name(TargetName) of
-	{ok,TargetRef} ->
+    case ct_config:get_key_from_name(TargetName) of
+	{ok,_Key} ->
 	    case ets:select(?conn_table,[{#conn{handle='$1',
-						targetref=TargetRef,
+						targetref=TargetName,
 						address=Address,
 						callback=Callback},
 					  [],
@@ -514,41 +518,76 @@ does_connection_exist(TargetName,Address,Callback) ->
     end.
 
 %%%-----------------------------------------------------------------
-%%% @spec get_connections(TargetName,Callback) -> 
-%%%                                {ok,Connections} | {error,Reason}
+%%% @spec get_connection(TargetName,Callback) -> 
+%%%                                {ok,Connection} | {error,Reason}
 %%%      TargetName = ct:target_name()
 %%%      Callback = atom()
-%%%      Connections = [Connection]
 %%%      Connection = {Handle,Address}
 %%%      Handle = term()
 %%%      Address = term()
 %%%
-%%% @doc Return all connections for the <code>Callback</code> on the
+%%% @doc Return the connection for <code>Callback</code> on the
 %%% given target (<code>TargetName</code>).
-get_connections(TargetName,Callback) ->
-    case ct_config:get_ref_from_name(TargetName) of
-	{ok,Ref} ->
-	    {ok,ets:select(?conn_table,[{#conn{handle='$1',
-					       address='$2',
-					       targetref=Ref,
-					       callback=Callback},
-					 [],
-					 [{{'$1','$2'}}]}])};
+get_connection(TargetName,Callback) ->
+    %% check that TargetName is a registered alias
+    case ct_config:get_key_from_name(TargetName) of
+	{ok,_Key} ->
+	    case ets:select(?conn_table,[{#conn{handle='$1',
+						address='$2',
+						targetref=TargetName,
+						callback=Callback},
+					  [],
+					  [{{'$1','$2'}}]}]) of
+		[Result] ->
+		    {ok,Result};
+		[] ->
+		    {error,no_registered_connection}
+	    end;
 	Error ->
 	    Error
     end.
 
 %%%-----------------------------------------------------------------
+%%% @spec get_connections(ConnPid) -> 
+%%%                                {ok,Connections} | {error,Reason}
+%%%      Connections = [Connection]
+%%%      Connection = {TargetName,Handle,Callback,Address}
+%%%      TargetName = ct:target_name() | undefined
+%%%      Handle = term()
+%%%      Callback = atom()
+%%%      Address = term()
+%%%
+%%% @doc Get data for all connections associated with a particular
+%%%      connection pid (see Callback:init/3).
+get_connections(ConnPid) ->
+    Conns = ets:tab2list(?conn_table),
+    lists:flatmap(fun(#conn{targetref=TargetName,
+			    handle=Handle,
+			    callback=Callback,
+			    address=Address}) ->
+			  case ct_gen_conn:get_conn_pid(Handle) of
+			      ConnPid when is_atom(TargetName) ->
+				  [{TargetName,Handle,
+				    Callback,Address}];
+			      ConnPid ->
+				  [{undefined,Handle,
+				   Callback,Address}];
+			      _ ->
+				  []
+			  end
+		  end, Conns).
+
+%%%-----------------------------------------------------------------
 %%% @hidden
 %%% @equiv ct:get_target_name/1
-get_target_name(ConnPid) ->
-    case ets:select(?conn_table,[{#conn{handle=ConnPid,targetref='$1',_='_'},
+get_target_name(Handle) ->
+    case ets:select(?conn_table,[{#conn{handle=Handle,targetref='$1',_='_'},
 				  [],
 				  ['$1']}]) of
-	[TargetRef] ->
-	    ct_config:get_name_from_ref(TargetRef);
-	[] ->
-	    {error,{unknown_connection,ConnPid}}
+	[TargetName] when is_atom(TargetName) ->
+	    {ok,TargetName};
+	_ ->
+	    {error,{unknown_connection,Handle}}
     end.
 
 %%%-----------------------------------------------------------------
@@ -922,29 +961,6 @@ cast(Msg) ->
 seconds(T) ->
     test_server:seconds(T).
 
-ct_make_ref() ->
-    Pid = case whereis(ct_make_ref) of
-	      undefined -> 
-		  spawn_link(fun() -> ct_make_ref_init() end);
-	      P -> 
-		  P
-	  end,
-    Pid ! {self(),ref_req},
-    receive
-	{Pid,Ref} -> Ref
-    end.
-
-ct_make_ref_init() ->
-    register(ct_make_ref,self()),
-    ct_make_ref_loop(0).
-
-ct_make_ref_loop(N) ->
-    receive
-	{From,ref_req} -> 
-	    From ! {self(),N},
-	    ct_make_ref_loop(N+1)
-    end.
-
 abs_name("/") ->
     "/";
 abs_name(Dir0) ->