Fix halfway-stopping of listeners

* if the process calling ranch:stop_listener crashes before finishing,
  the stopping procedure is still executed completely
* if a listener is terminated but not deleted, calling ranch:stop_listener
  removes the remnant

2 files changed, 46 insertions, 16 deletions

diff --git a/src/ranch.erl b/src/ranch.erl
index d48c18a..85c2c73 100644
--- a/src/ranch.erl
+++ b/src/ranch.erl
@@ -193,24 +193,38 @@ start_error(_, Error) -> Error.
 
 -spec stop_listener(ref()) -> ok | {error, not_found}.
 stop_listener(Ref) ->
+	%% The stop procedure must be executed in a separate
+	%% process to make sure that it won't be interrupted
+	%% in the middle in case the calling process crashes.
+	%% We use erpc:call locally so we don't have to
+	%% implement a custom spawn/call mechanism.
+	%% We need to provide an integer timeout to erpc:call,
+	%% otherwise the function will be executed in the calling
+	%% process. 5 minutes should be enough.
+	erpc:call(node(), fun() -> stop_listener1(Ref) end, 300000).
+
+stop_listener1(Ref) ->
+	TransportAndOpts = maybe_get_transport_and_opts(Ref),
+	_ = supervisor:terminate_child(ranch_sup, {ranch_listener_sup, Ref}),
+	ok = ranch_server:cleanup_listener_opts(Ref),
+	Result = supervisor:delete_child(ranch_sup, {ranch_listener_sup, Ref}),
+	ok = stop_listener2(TransportAndOpts),
+	Result.
+
+stop_listener2({Transport, TransOpts}) ->
+	Transport:cleanup(TransOpts),
+	ok;
+stop_listener2(undefined) ->
+	ok.
+
+maybe_get_transport_and_opts(Ref) ->
 	try
-		[_, Transport0, _, _, _] = ranch_server:get_listener_start_args(Ref),
-		TransOpts0 = get_transport_options(Ref),
-		{Transport0, TransOpts0}
-	of
-		{Transport, TransOpts} ->
-			case supervisor:terminate_child(ranch_sup, {ranch_listener_sup, Ref}) of
-				ok ->
-					_ = supervisor:delete_child(ranch_sup, {ranch_listener_sup, Ref}),
-					ranch_server:cleanup_listener_opts(Ref),
-					Transport:cleanup(TransOpts),
-					ok;
-				{error, Reason} ->
-					{error, Reason}
-			end
+		[_, Transport, _, _, _] = ranch_server:get_listener_start_args(Ref),
+		TransOpts = get_transport_options(Ref),
+		{Transport, TransOpts}
 	catch
 		error:badarg ->
-			{error, not_found}
+			undefined
 	end.
 
 -spec suspend_listener(ref()) -> ok | {error, any()}.
diff --git a/test/acceptor_SUITE.erl b/test/acceptor_SUITE.erl
index ba09697..7144612 100644
--- a/test/acceptor_SUITE.erl
+++ b/test/acceptor_SUITE.erl
@@ -112,7 +112,8 @@ groups() ->
 		misc_wait_for_connections,
 		misc_multiple_ip_local_socket_opts,
 		misc_connection_alarms,
-		misc_stop_unknown_listener
+		misc_stop_unknown_listener,
+		misc_repeated_start_stop
 	]}, {supervisor, [
 		connection_type_supervisor,
 		connection_type_supervisor_separate_from_connection,
@@ -665,6 +666,21 @@ misc_stop_unknown_listener(_) ->
 	{error, not_found} = ranch:stop_listener(make_ref()),
 	ok.
 
+misc_repeated_start_stop(_) ->
+	doc("Ensure that repeated starts and stops of a listener works."),
+	Name = name(),
+	lists:foreach(
+		fun(_) ->
+			{ok, _} = ranch:start_listener(Name, ranch_tcp, #{}, echo_protocol, []),
+			true = is_integer(ranch:get_port(Name)),
+			ok = ranch:stop_listener(Name),
+			{'EXIT', _} = begin catch ranch:get_port(Name) end
+		end,
+		lists:seq(1, 10)
+	),
+	ok.
+
+
 %% ssl.
 
 ssl_accept_error(_) ->