From 7d77cc5a9e61fbe39f0574014926064723523efa Mon Sep 17 00:00:00 2001 From: Peter Andersson Date: Wed, 29 May 2013 01:03:05 +0200 Subject: Fix faulty connection handling OTP-10126 --- lib/common_test/src/ct_config.erl | 33 ++------- lib/common_test/src/ct_ftp.erl | 6 +- lib/common_test/src/ct_gen_conn.erl | 37 +++++++--- lib/common_test/src/ct_netconfc.erl | 8 +-- lib/common_test/src/ct_ssh.erl | 6 +- lib/common_test/src/ct_telnet.erl | 17 ++--- lib/common_test/src/ct_util.erl | 132 ++++++++++++++++++++---------------- 7 files changed, 122 insertions(+), 117 deletions(-) (limited to 'lib/common_test/src') 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 1339e53780..3645be52d5 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 Callback:close/1.

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 Callback on the +%%% @doc Return the connection for Callback on the %%% given target (TargetName). -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) -> -- cgit v1.2.3