Browse Source

Check the accept/2 return value for errors

Distinguish the errors from transport_accept and ssl_accept
in ranch_ssl. {error, closed} for the first one means the listening
socket got closed; for the second one it means the connection
socket was.

Ignore all errors except when the listening socket got closed,
where we want to crash to allow opening the socket again.
Loïc Hoguin 13 years ago
parent
commit
eabb029895
3 changed files with 34 additions and 7 deletions
  1. 14 4
      src/ranch_acceptor.erl
  2. 3 3
      src/ranch_ssl.erl
  3. 17 0
      test/acceptor_SUITE.erl

+ 14 - 4
src/ranch_acceptor.erl

@@ -46,6 +46,11 @@ init(LSocket, Transport, Protocol, MaxConns, Opts, ListenerPid, ConnsSup) ->
 	non_neg_integer(), any(), pid(), pid()) -> no_return().
 	non_neg_integer(), any(), pid(), pid()) -> no_return().
 loop(LSocket, Transport, Protocol, MaxConns, Opts, ListenerPid, ConnsSup) ->
 loop(LSocket, Transport, Protocol, MaxConns, Opts, ListenerPid, ConnsSup) ->
 	receive
 	receive
+		%% We couldn't accept the socket but it's safe to continue.
+		{accept, continue} ->
+			?MODULE:init(LSocket, Transport, Protocol,
+				MaxConns, Opts, ListenerPid, ConnsSup);
+		%% Found my sockets!
 		{accept, CSocket} ->
 		{accept, CSocket} ->
 			{ok, ConnPid} = supervisor:start_child(ConnsSup,
 			{ok, ConnPid} = supervisor:start_child(ConnsSup,
 				[ListenerPid, CSocket, Transport, Protocol, Opts]),
 				[ListenerPid, CSocket, Transport, Protocol, Opts]),
@@ -55,6 +60,7 @@ loop(LSocket, Transport, Protocol, MaxConns, Opts, ListenerPid, ConnsSup) ->
 			maybe_wait(ListenerPid, MaxConns, NbConns),
 			maybe_wait(ListenerPid, MaxConns, NbConns),
 			?MODULE:init(LSocket, Transport, Protocol,
 			?MODULE:init(LSocket, Transport, Protocol,
 				MaxConns, Opts, ListenerPid, ConnsSup);
 				MaxConns, Opts, ListenerPid, ConnsSup);
+		%% Upgrade the protocol options.
 		{set_opts, Opts2} ->
 		{set_opts, Opts2} ->
 			?MODULE:loop(LSocket, Transport, Protocol,
 			?MODULE:loop(LSocket, Transport, Protocol,
 				MaxConns, Opts2, ListenerPid, ConnsSup)
 				MaxConns, Opts2, ListenerPid, ConnsSup)
@@ -72,9 +78,13 @@ maybe_wait(ListenerPid, MaxConns, _) ->
 async_accept(LSocket, Transport) ->
 async_accept(LSocket, Transport) ->
 	AcceptorPid = self(),
 	AcceptorPid = self(),
 	_ = spawn_link(fun() ->
 	_ = spawn_link(fun() ->
-		%% @todo {error, closed} must be handled and other errors ignored.
-		{ok, CSocket} = Transport:accept(LSocket, infinity),
-		Transport:controlling_process(CSocket, AcceptorPid),
-		AcceptorPid ! {accept, CSocket}
+		case Transport:accept(LSocket, infinity) of
+			{ok, CSocket} ->
+				Transport:controlling_process(CSocket, AcceptorPid),
+				AcceptorPid ! {accept, CSocket};
+			%% We want to crash if the listening socket got closed.
+			{error, Reason} when Reason =/= closed ->
+				AcceptorPid ! {accept, continue}
+		end
 	end),
 	end),
 	ok.
 	ok.

+ 3 - 3
src/ranch_ssl.erl

@@ -111,7 +111,7 @@ listen(Opts) ->
 %% @see ssl:transport_accept/2
 %% @see ssl:transport_accept/2
 %% @see ssl:ssl_accept/2
 %% @see ssl:ssl_accept/2
 -spec accept(ssl:sslsocket(), timeout())
 -spec accept(ssl:sslsocket(), timeout())
-	-> {ok, ssl:sslsocket()} | {error, closed | timeout | atom()}.
+	-> {ok, ssl:sslsocket()} | {error, closed | timeout | atom() | tuple()}.
 accept(LSocket, Timeout) ->
 accept(LSocket, Timeout) ->
 	case ssl:transport_accept(LSocket, Timeout) of
 	case ssl:transport_accept(LSocket, Timeout) of
 		{ok, CSocket} ->
 		{ok, CSocket} ->
@@ -179,11 +179,11 @@ require([App|Tail]) ->
 	require(Tail).
 	require(Tail).
 
 
 -spec ssl_accept(ssl:sslsocket(), timeout())
 -spec ssl_accept(ssl:sslsocket(), timeout())
-	-> {ok, ssl:sslsocket()} | {error, closed | timeout | atom()}.
+	-> {ok, ssl:sslsocket()} | {error, {ssl_accept, atom()}}.
 ssl_accept(Socket, Timeout) ->
 ssl_accept(Socket, Timeout) ->
 	case ssl:ssl_accept(Socket, Timeout) of
 	case ssl:ssl_accept(Socket, Timeout) of
 		ok ->
 		ok ->
 			{ok, Socket};
 			{ok, Socket};
 		{error, Reason} ->
 		{error, Reason} ->
-			{error, Reason}
+			{error, {ssl_accept, Reason}}
 	end.
 	end.

+ 17 - 0
test/acceptor_SUITE.erl

@@ -25,6 +25,7 @@
 -export([end_per_group/2]).
 -export([end_per_group/2]).
 
 
 %% ssl.
 %% ssl.
+-export([ssl_accept_error/1]).
 -export([ssl_echo/1]).
 -export([ssl_echo/1]).
 
 
 %% tcp.
 %% tcp.
@@ -45,6 +46,7 @@ groups() ->
 		tcp_max_connections_and_beyond,
 		tcp_max_connections_and_beyond,
 		tcp_upgrade
 		tcp_upgrade
 	]}, {ssl, [
 	]}, {ssl, [
+		ssl_accept_error,
 		ssl_echo
 		ssl_echo
 	]}].
 	]}].
 
 
@@ -74,6 +76,21 @@ end_per_group(_, _) ->
 
 
 %% ssl.
 %% ssl.
 
 
+ssl_accept_error(Config) ->
+	{ok, _} = ranch:start_listener(ssl_accept_error, 1,
+		ranch_ssl, [{port, 0},
+			{certfile, ?config(data_dir, Config) ++ "cert.pem"}],
+		echo_protocol, []),
+	Port = ranch:get_port(ssl_accept_error),
+	[AcceptorPid] = ets:lookup_element(ranch_server,
+		{acceptors, ssl_accept_error}, 2),
+	true = is_process_alive(AcceptorPid),
+	{ok, Socket} = gen_tcp:connect("localhost", Port,
+		[binary, {active, false}, {packet, raw}]),
+	ok = gen_tcp:close(Socket),
+	receive after 500 -> ok end,
+	true = is_process_alive(AcceptorPid).
+
 ssl_echo(Config) ->
 ssl_echo(Config) ->
 	{ok, _} = ranch:start_listener(ssl_echo, 1,
 	{ok, _} = ranch:start_listener(ssl_echo, 1,
 		ranch_ssl, [{port, 0},
 		ranch_ssl, [{port, 0},