Browse Source

Remove socket option

j.uhlig 7 years ago
parent
commit
7cbc7fed32

+ 16 - 6
doc/src/guide/listeners.asciidoc

@@ -162,12 +162,22 @@ solutions is however out of the scope of this guide.
 
 === Accepting connections on an existing socket
 
-If you want to accept connections on an existing socket, you can use the
-`socket` transport option, which should just be the relevant data returned
-from the connect function for the transport or the underlying socket library
-(`gen_tcp:connect`, `ssl:connect`). The accept function will then be
-called on the passed in socket. You should connect the socket in
-`{active, false}` mode, as well.
+If you want to accept connections on an existing socket, you can write
+a custom `ranch_transport` implementation that fetches or otherwise
+acquires a listening socket in the `listen/1` callback and returns it
+in the form of `{ok, ListenSocket}`.
+
+The custom `listen/1` function must ensure that the listener process
+(usually the process calling it) is also made the controlling process
+of the socket it returns. Failing to do so will result in stop/start
+and suspend/resume not working properly for that listener.
+
+While it might be tempting to pass in an existing socket to your
+custom transport by way of the transport options, thereby mimicking
+a deprecated ranch behavior, this is considered dangerous and was removed
+for that very reason. In general, it should be ensured that the socket
+to be returned from a custom `listen/1` function really is a listening
+socket and that it is operational.
 
 === Limiting the number of concurrent connections
 

+ 1 - 9
doc/src/manual/ranch.asciidoc

@@ -73,7 +73,6 @@ opt() = {ack_timeout, timeout()}
       | {max_connections, max_conns()}
       | {num_acceptors, pos_integer()}
       | {shutdown, timeout() | brutal_kill}
-      | {socket, any()}
 ----
 
 Deprecated form for Ranch-specific options.
@@ -93,7 +92,6 @@ opts() = any() | #{
     logger            => module(),
     num_acceptors     => pos_integer(),
     shutdown          => timeout() | brutal_kill,
-    socket            => any(),
     socket_opts       => any()
 }
 ----
@@ -135,13 +133,6 @@ shutdown (5000)::
 
 Maximum allowed time for children to stop on listener shutdown.
 
-socket::
-
-Listening socket opened externally to be used instead of
-calling `Transport:listen/1`. This option will be removed
-in Ranch 2.0. Use a custom transport module acting as a
-wrapper for `ranch_tcp` or `ranch_ssl` instead.
-
 socket_opts::
 
 Socket options given to `Transport:listen/1`. Please refer to the
@@ -158,6 +149,7 @@ Unique name used to refer to a listener.
 
 == Changelog
 
+* *2.0*: The `socket` option was removed.
 * *1.6*: The `logger` option was added.
 * *1.6*: The `opt()` type was deprecated in favor of the new `opts()` type.
 

+ 4 - 22
src/ranch.erl

@@ -56,8 +56,7 @@
 	| {connection_type, worker | supervisor}
 	| {max_connections, max_conns()}
 	| {num_acceptors, pos_integer()}
-	| {shutdown, timeout() | brutal_kill}
-	| {socket, any()}.
+	| {shutdown, timeout() | brutal_kill}.
 -export_type([opt/0]).
 
 -type opts() :: any() | #{
@@ -67,7 +66,6 @@
 	logger => module(),
 	num_acceptors => pos_integer(),
 	shutdown => timeout() | brutal_kill,
-	socket => any(),
 	socket_opts => any()
 }.
 -export_type([opts/0]).
@@ -85,24 +83,8 @@ start_listener(Ref, Transport, TransOpts0, Protocol, ProtoOpts)
 		false ->
 			{error, badarg};
 		true ->
-			Res = supervisor:start_child(ranch_sup, child_spec(Ref,
-					Transport, TransOpts, Protocol, ProtoOpts)),
-			Socket = maps:get(socket, TransOpts, undefined),
-			case Res of
-				{ok, Pid} when Socket =/= undefined ->
-					%% Give ownership of the socket to ranch_acceptors_sup
-					%% to make sure the socket stays open as long as the
-					%% listener is alive. If the socket closes however there
-					%% will be no way to recover because we don't know how
-					%% to open it again.
-					Children = supervisor:which_children(Pid),
-					{_, AcceptorsSup, _, _}
-						= lists:keyfind(ranch_acceptors_sup, 1, Children),
-					Transport:controlling_process(Socket, AcceptorsSup);
-				_ ->
-					ok
-			end,
-			maybe_started(Res)
+			maybe_started(supervisor:start_child(ranch_sup, child_spec(Ref,
+					Transport, TransOpts, Protocol, ProtoOpts)))
 	end.
 
 -spec start_listener(ref(), non_neg_integer(), module(), opts(), module(), any())
@@ -131,7 +113,7 @@ normalize_opts(List0) when is_list(List0) ->
 			false ->
 				{Map2, List2}
 		end
-	end, {Map1, List1}, [connection_type, max_connections, num_acceptors, shutdown, socket]),
+	end, {Map1, List1}, [connection_type, max_connections, num_acceptors, shutdown]),
 	if
 		Map =:= #{} ->
 			ok;

+ 12 - 17
src/ranch_acceptors_sup.erl

@@ -28,23 +28,18 @@ init([Ref, Transport]) ->
 	TransOpts = ranch_server:get_transport_options(Ref),
 	NumAcceptors = maps:get(num_acceptors, TransOpts, 10),
 	Logger = maps:get(logger, TransOpts, error_logger),
-	LSocket = case maps:get(socket, TransOpts, undefined) of
-		undefined ->
-			SocketOpts = maps:get(socket_opts, TransOpts, []),
-			%% We temporarily put the logger in the process dictionary
-			%% so that it can be used from ranch:filter_options. The
-			%% interface as it currently is does not allow passing it
-			%% down otherwise.
-			put(logger, Logger),
-			case Transport:listen(SocketOpts) of
-				{ok, Socket} ->
-					erase(logger),
-					Socket;
-				{error, Reason} ->
-					listen_error(Ref, Transport, SocketOpts, Reason, Logger)
-			end;
-		Socket ->
-			Socket
+	SocketOpts = maps:get(socket_opts, TransOpts, []),
+	%% We temporarily put the logger in the process dictionary
+	%% so that it can be used from ranch:filter_options. The
+	%% interface as it currently is does not allow passing it
+	%% down otherwise.
+	put(logger, Logger),
+	LSocket = case Transport:listen(SocketOpts) of
+		{ok, Socket} ->
+			erase(logger),
+			Socket;
+		{error, Reason} ->
+			listen_error(Ref, Transport, SocketOpts, Reason, Logger)
 	end,
 	{ok, Addr} = Transport:sockname(LSocket),
 	ranch_server:set_addr(Ref, Addr),

+ 0 - 37
test/acceptor_SUITE.erl

@@ -28,7 +28,6 @@ all() ->
 
 groups() ->
 	[{tcp, [
-		tcp_accept_socket,
 		tcp_active_echo,
 		tcp_echo,
 		tcp_graceful,
@@ -47,7 +46,6 @@ groups() ->
 		tcp_error_eacces
 	]}, {ssl, [
 		ssl_accept_error,
-		ssl_accept_socket,
 		ssl_active_echo,
 		ssl_echo,
 		ssl_graceful,
@@ -399,24 +397,6 @@ ssl_accept_error(_) ->
 	true = is_process_alive(AcceptorPid),
 	ok = ranch:stop_listener(Name).
 
-ssl_accept_socket(_) ->
-	doc("Ensure that listener can use an externally opened SSL listen socket."),
-	Name = name(),
-	Opts = ct_helper:get_certs_from_ets(),
-	{ok, S} = ssl:listen(0, [binary, {active, false}, {packet, raw}, {reuseaddr, true}|Opts]),
-	{ok, _} = ranch:start_listener(Name,
-		ranch_ssl, #{socket => S},
-		echo_protocol, []),
-	Port = ranch:get_port(Name),
-	{ok, Socket} = ssl:connect("localhost", Port, [binary, {active, false}, {packet, raw}]),
-	ok = ssl:send(Socket, <<"TCP Ranch is working!">>),
-	{ok, <<"TCP Ranch is working!">>} = ssl:recv(Socket, 21, 1000),
-	ok = ranch:stop_listener(Name),
-	{error, closed} = ssl:recv(Socket, 0, 1000),
-	%% Make sure the listener stopped.
-	{'EXIT', _} = begin catch ranch:get_port(Name) end,
-	ok.
-
 ssl_active_echo(_) ->
 	doc("Ensure that active mode works with SSL transport."),
 	Name = name(),
@@ -656,23 +636,6 @@ ssl_error_eacces(_) ->
 
 %% tcp.
 
-tcp_accept_socket(_) ->
-	doc("Ensure that listener can use an externally opened TCP listen socket."),
-	Name = name(),
-	{ok, S} = gen_tcp:listen(0, [binary, {active, false}, {packet, raw}, {reuseaddr, true}]),
-	{ok, _} = ranch:start_listener(Name,
-		ranch_tcp, #{socket => S},
-		echo_protocol, []),
-	Port = ranch:get_port(Name),
-	{ok, Socket} = gen_tcp:connect("localhost", Port, [binary, {active, false}, {packet, raw}]),
-	ok = gen_tcp:send(Socket, <<"TCP Ranch is working!">>),
-	{ok, <<"TCP Ranch is working!">>} = gen_tcp:recv(Socket, 21, 1000),
-	ok = ranch:stop_listener(Name),
-	{error, closed} = gen_tcp:recv(Socket, 0, 1000),
-	%% Make sure the listener stopped.
-	{'EXIT', _} = begin catch ranch:get_port(Name) end,
-	ok.
-
 tcp_active_echo(_) ->
 	doc("Ensure that active mode works with TCP transport."),
 	Name = name(),