[common_test] Unregister connection if killed

The symptom of this bug was that a (named) telnet connection which was
open and hanging during a timetrap timeout could no longer be opened
again in subsequent test cases.

Since the connection was hanging, ct_telnet:close/1 which was called
during end_per_testcase would fail (timeout) and cause common_test to
brutally kill the connection. The bug was that the connection was not
unregistered at this point. When trying to open the connection again
in the next testcase, common_test attempted to reuse the same
connection since it had not been unregistered. This failed since the
connection in fact was dead.

3 files changed, 91 insertions, 2 deletions

diff --git a/lib/common_test/src/ct_util.erl b/lib/common_test/src/ct_util.erl
index 6a8b37bf3b..b77845eb5b 100644
--- a/lib/common_test/src/ct_util.erl
+++ b/lib/common_test/src/ct_util.erl
@@ -421,6 +421,8 @@ loop(Mode,TestData,StartDir) ->
 					 "Reason: ~p\n\n",
 					 [Pid,A,CB,Reason]),
 		    catch CB:close(Pid),
+		    %% in case CB:close failed to do this:
+		    unregister_connection(Pid),
 		    loop(Mode,TestData,StartDir);
 		_ ->
 		    %% Let process crash in case of error, this shouldn't happen!
diff --git a/lib/common_test/test/ct_telnet_SUITE.erl b/lib/common_test/test/ct_telnet_SUITE.erl
index 17617f59eb..e2ee207754 100644
--- a/lib/common_test/test/ct_telnet_SUITE.erl
+++ b/lib/common_test/test/ct_telnet_SUITE.erl
@@ -72,7 +72,8 @@ suite() -> [{ct_hooks,[ts_install_cth]}].
 all() ->
     [
      unix_telnet,
-     own_server
+     own_server,
+     timetrap
     ].
 
 %%--------------------------------------------------------------------
@@ -88,6 +89,10 @@ own_server(Config) ->
     all_tests_in_suite(own_server,"ct_telnet_own_server_SUITE",
 		       "telnet2.cfg",Config).
 
+timetrap(Config) ->
+    all_tests_in_suite(timetrap,"ct_telnet_timetrap_SUITE",
+		       "telnet3.cfg",Config).
+
 %%%-----------------------------------------------------------------
 %%% HELP FUNCTIONS
 %%%-----------------------------------------------------------------
@@ -143,7 +148,13 @@ telnet_config(_) ->
 events_to_check(unix_telnet,Config) ->
     all_cases(ct_telnet_basic_SUITE,Config);
 events_to_check(own_server,Config) ->
-    all_cases(ct_telnet_own_server_SUITE,Config).
+    all_cases(ct_telnet_own_server_SUITE,Config);
+events_to_check(timetrap,_Config) ->
+    [{?eh,start_logging,{'DEF','RUNDIR'}},
+     {?eh,tc_done,{ct_telnet_timetrap_SUITE,expect_timetrap,
+		   {failed,{timetrap_timeout,7000}}}},
+     {?eh,tc_done,{ct_telnet_timetrap_SUITE,expect_success,ok}},
+     {?eh,stop_logging,[]}].
 
 all_cases(Suite,Config) ->
     {module,_} = code:load_abs(filename:join(?config(data_dir,Config),
diff --git a/lib/common_test/test/ct_telnet_SUITE_data/ct_telnet_timetrap_SUITE.erl b/lib/common_test/test/ct_telnet_SUITE_data/ct_telnet_timetrap_SUITE.erl
new file mode 100644
index 0000000000..f274fb9112
--- /dev/null
+++ b/lib/common_test/test/ct_telnet_SUITE_data/ct_telnet_timetrap_SUITE.erl
@@ -0,0 +1,76 @@
+-module(ct_telnet_timetrap_SUITE).
+
+-compile(export_all).
+
+-include_lib("common_test/include/ct.hrl").
+
+-define(name,erl_telnet_server).
+
+%%--------------------------------------------------------------------
+%% TEST SERVER CALLBACK FUNCTIONS
+%%--------------------------------------------------------------------
+
+init_per_suite(Config) ->
+    Config.
+
+end_per_suite(_Config) ->
+    ok.
+
+suite() -> [{require,?name,{unix,[telnet]}},
+	    {timetrap,{seconds,7}}].
+
+all() ->
+    [expect_timetrap,
+     expect_success].
+
+groups() ->
+    [].
+
+init_per_group(_GroupName, Config) ->
+    Config.
+
+end_per_group(_GroupName, Config) ->
+    Config.
+
+init_per_testcase(_,Config) ->
+    ct:log("init_per_testcase: opening telnet connection...",[]),
+    {ok,_} = ct_telnet:open(?name),
+    ct:log("...done",[]),
+    Config.
+
+end_per_testcase(_,_Config) ->
+    ct:log("end_per_testcase: closing telnet connection...",[]),
+    _ = ct_telnet:close(?name),
+    ct:log("...done",[]),
+    ok.
+
+
+%% OTP-10648
+%% This test case should fail with timetrap timeout.
+%%
+%% The long timetrap timeout and timeout option in the expect call
+%% also causes the telnet client to hang so long that the attempt at
+%% closing it (in end_per_testcase) will time out (close timeout is 5
+%% sec) without a timetrap timeout occuring in end_per_testcase.
+%%
+%% The point is to see that the connection is thoroughly removed and
+%% unregistered anyway so that the next test case can successfully
+%% open a connection with the same name.
+%%
+%% Note!!! that if end_per_testcase reaches a timetrap timeout before
+%% the connection is closed, then the connection will survive until
+%% the hanging expect times out, after which it will be closed in a
+%% seemingly normal way due to the close message which was sent by the
+%% close attempt. This could happen any time during the subsequent
+%% test cases and cause confusion... There is however not much to do
+%% about this, except writing test cases with timetrap timeout longer
+%% than the close timeout (as this test case does)
+expect_timetrap(_) ->
+    {error,timeout} = ct_telnet:expect(?name, ["ayt"], [{timeout,20000}]),
+    ok.
+
+%% This should succeed
+expect_success(_) ->
+    ok = ct_telnet:send(?name, "echo ayt"),
+    {ok,["ayt"]} = ct_telnet:expect(?name, ["ayt"]),
+    ok.