File 1222-ssl-Use-significante-child-for-TLS-connect... of Package erlang

Overview Repositories Revisions Requests Users Attributes Meta

File 1222-ssl-Use-significante-child-for-TLS-connections.patch of Package erlang (Revision a3c4263f873c2273804c376249a3c22f)

Currently displaying revision a3c4263f873c2273804c376249a3c22f , Show latest

From 3f8d48a6fa0dd5ccdf6c0d401efca52228af5ca8 Mon Sep 17 00:00:00 2001
From: Ingela Anderton Andin <ingela@erlang.org>
Date: Wed, 27 Oct 2021 15:33:11 +0200
Subject: [PATCH 2/2]  ssl: Use significante child for TLS connections

This will allow the supervisor tree to manage all close scenarios and
we can remove special code that handled this manually before significant
child functionality was introduced.
---
 lib/ssl/src/Makefile                   |  1 +
 lib/ssl/src/ssl.app.src                |  1 +
 lib/ssl/src/ssl_gen_statem.erl         |  6 +-
 lib/ssl/src/tls_connection.erl         | 16 +-----
 lib/ssl/src/tls_connection_sup.erl     | 15 +++--
 lib/ssl/src/tls_dyn_connection_sup.erl | 78 ++++++++++++++++++++++++++
 lib/ssl/src/tls_gen_connection.erl     | 17 +++---
 lib/ssl/src/tls_sender.erl             | 43 +++++++++-----
 lib/ssl/src/tls_socket.erl             | 23 +++++---
 lib/ssl/test/tls_api_SUITE.erl         | 35 ++++++------
 10 files changed, 160 insertions(+), 75 deletions(-)
 create mode 100644 lib/ssl/src/tls_dyn_connection_sup.erl

diff --git a/lib/ssl/src/Makefile b/lib/ssl/src/Makefile
index 9cbee91946..1ba0f49f91 100644
--- a/lib/ssl/src/Makefile
+++ b/lib/ssl/src/Makefile
@@ -103,6 +103,7 @@ MODULES= \
 	tls_client_ticket_store \
 	tls_dist_sup \
 	tls_dist_server_sup \
+	tls_dyn_connection_sup\
 	tls_sender \
 	tls_server_session_ticket\
 	tls_server_session_ticket_sup\
diff --git a/lib/ssl/src/ssl.app.src b/lib/ssl/src/ssl.app.src
index d761520856..a43a84f26d 100644
--- a/lib/ssl/src/ssl.app.src
+++ b/lib/ssl/src/ssl.app.src
@@ -18,6 +18,7 @@
                tls_server_session_ticket_sup,
                tls_server_session_ticket,
                tls_sup,
+               tls_dyn_connection_sup,
 	       ssl_dh_groups,
 	       %% DTLS
 	       dtls_connection,
diff --git a/lib/ssl/src/ssl_gen_statem.erl b/lib/ssl/src/ssl_gen_statem.erl
index 46926d15ef..3bbb820ef7 100644
--- a/lib/ssl/src/ssl_gen_statem.erl
+++ b/lib/ssl/src/ssl_gen_statem.erl
@@ -126,9 +126,8 @@ start_link(Role, Host, Port, Socket, Options, User, CbInfo) ->
 -spec init(list()) -> no_return().
 %% Description: Initialization
 %%--------------------------------------------------------------------
-init([_Role, Sender, _Host, _Port, _Socket, {#{erl_dist := ErlDist} = TLSOpts, _, _},  _User, _CbInfo] = InitArgs) ->
+init([_Role, _Sender, _Host, _Port, _Socket, {#{erl_dist := ErlDist} = TLSOpts, _, _},  _User, _CbInfo] = InitArgs) ->
     process_flag(trap_exit, true),
-    link(Sender),
     case ErlDist of
         true ->
             process_flag(priority, max);
@@ -1191,11 +1190,12 @@ call(FsmPid, Event) ->
 	    {error, closed};
 	exit:{normal, _} ->
 	    {error, closed};
+	exit:{shutdown,_} ->
+	    {error, closed};
 	exit:{{shutdown, _},_} ->
 	    {error, closed}
     end.
 
-
 check_hostname(_, "") ->
     ?ALERT_REC(?FATAL, ?UNRECOGNIZED_NAME, empty_sni);
 
diff --git a/lib/ssl/src/tls_connection.erl b/lib/ssl/src/tls_connection.erl
index 162390b74b..ed2183c826 100644
--- a/lib/ssl/src/tls_connection.erl
+++ b/lib/ssl/src/tls_connection.erl
@@ -436,8 +436,7 @@ terminate({shutdown, {sender_died, Reason}}, _StateName,
     ssl_gen_statem:handle_trusted_certs_db(State),
     tls_gen_connection:close(Reason, Socket, Transport, undefined, undefined);
 terminate(Reason, StateName, State) ->
-    catch ssl_gen_statem:terminate(Reason, StateName, State),
-    ensure_sender_terminate(Reason, State).
+    ssl_gen_statem:terminate(Reason, StateName, State).
 
 format_status(Type, Data) ->
     ssl_gen_statem:format_status(Type, Data).
@@ -553,19 +552,6 @@ gen_info(Event, StateName, State) ->
 					    StateName, State)
     end.
 
-ensure_sender_terminate(downgrade, _) ->
-    ok; %% Do not terminate sender during downgrade phase 
-ensure_sender_terminate(_,  #state{protocol_specific = #{sender := Sender}}) ->
-    %% Make sure TLS sender dies when connection process is terminated normally
-    %% This is needed if the tls_sender is blocked in prim_inet:send 
-    Kill = fun() -> 
-                   receive 
-                   after 5000 ->
-                           catch (exit(Sender, kill))
-                   end
-           end,
-    spawn(Kill).
-
 choose_tls_fsm(#{versions := Versions},
                #client_hello{
                   extensions = #{client_hello_versions :=
diff --git a/lib/ssl/src/tls_connection_sup.erl b/lib/ssl/src/tls_connection_sup.erl
index e08e682534..435639cf68 100644
--- a/lib/ssl/src/tls_connection_sup.erl
+++ b/lib/ssl/src/tls_connection_sup.erl
@@ -44,7 +44,7 @@ start_link_dist() ->
 
 start_child(Args) ->
     supervisor:start_child(?MODULE, Args).
-    
+
 start_child_dist(Args) ->
     supervisor:start_child(tls_dist_connection_sup, Args).
     
@@ -56,11 +56,10 @@ init(_) ->
                  intensity =>   0,
                  period    => 3600
                 },
-    ChildSpecs = [#{id       => undefined,
-                    start    => {ssl_gen_statem, start_link, []},
-                    restart  => temporary, 
-                    shutdown => 4000,
-                    modules  => [ssl_gen_statem, tls_connection, tls_connection_1_3],
-                    type     => worker
-                   }],    
+    ChildSpecs = [#{id      => undefined,
+                    restart => temporary,
+                    type    => supervisor,
+                    start   => {tls_dyn_connection_sup, start_link, []}}],
     {ok, {SupFlags, ChildSpecs}}.
+
+
diff --git a/lib/ssl/src/tls_dyn_connection_sup.erl b/lib/ssl/src/tls_dyn_connection_sup.erl
new file mode 100644
index 0000000000..cb0576cbd6
--- /dev/null
+++ b/lib/ssl/src/tls_dyn_connection_sup.erl
@@ -0,0 +1,78 @@
+%%
+%% %CopyrightBegin%
+%% 
+%% Copyright Ericsson AB 2021-2021. All Rights Reserved.
+%% 
+%% Licensed under the Apache License, Version 2.0 (the "License");
+%% you may not use this file except in compliance with the License.
+%% You may obtain a copy of the License at
+%%
+%%     http://www.apache.org/licenses/LICENSE-2.0
+%%
+%% Unless required by applicable law or agreed to in writing, software
+%% distributed under the License is distributed on an "AS IS" BASIS,
+%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+%% See the License for the specific language governing permissions and
+%% limitations under the License.
+%% 
+%% %CopyrightEnd%
+%%
+
+%%
+%%----------------------------------------------------------------------
+%% Purpose: Supervises the TLS generic state machine, a process that
+%% owns the transport socket and hence is a significant child, and the
+%% corresponding TLS sender process that sends data to avoid blocking
+%% the state machine process.
+%% ----------------------------------------------------------------------
+
+-module(tls_dyn_connection_sup).
+
+-behaviour(supervisor).
+
+%% API
+-export([start_link/0]).
+-export([start_child/3]).
+
+%% Supervisor callback
+-export([init/1]).
+
+%%%=========================================================================
+%%%  API
+%%%=========================================================================
+start_link() ->
+    supervisor:start_link(?MODULE, []).
+
+start_child(Sup, sender, Args) ->
+    supervisor:start_child(Sup, sender(Args));
+start_child(Sup, receiver, Args) ->
+    supervisor:start_child(Sup, receiver(Args)).
+    
+%%%=========================================================================
+%%%  Supervisor callback
+%%%=========================================================================
+init(_) ->
+    SupFlags = #{strategy      => one_for_all,
+                 auto_shutdown => any_significant,
+                 intensity     =>    0,
+                 period        => 3600
+                },
+    ChildSpecs = [],
+    {ok, {SupFlags, ChildSpecs}}.
+
+sender(Args) ->
+    #{id          => sender,
+      restart     => temporary,
+      type        => worker,
+      start       => {tls_sender, start_link, Args},
+      modules     => [tls_sender]
+     }.
+
+receiver(Args) ->
+    #{id          => receiver,
+      restart     => temporary,
+      type        => worker,
+      significant => true,
+      start       => {ssl_gen_statem, start_link, Args},
+      modules     => [ssl_gen_statem, tls_connection, tls_connection_1_3]
+     }.
diff --git a/lib/ssl/src/tls_gen_connection.erl b/lib/ssl/src/tls_gen_connection.erl
index dfb67a8f65..59d30d7cb0 100644
--- a/lib/ssl/src/tls_gen_connection.erl
+++ b/lib/ssl/src/tls_gen_connection.erl
@@ -82,9 +82,10 @@ start_fsm(Role, Host, Port, Socket, {#{erl_dist := false},_, Trackers} = Opts,
 	  User, {CbModule, _,_, _, _} = CbInfo, 
 	  Timeout) -> 
     try 
-        {ok, Sender} = tls_sender:start(),
-	{ok, Pid} = tls_connection_sup:start_child([Role, Sender, Host, Port, Socket, 
-						    Opts, User, CbInfo]), 
+        {ok, DynSup} =  tls_connection_sup:start_child([]),
+        {ok, Sender} = tls_dyn_connection_sup:start_child(DynSup, sender, []),
+	{ok, Pid} = tls_dyn_connection_sup:start_child(DynSup, receiver, [Role, Sender, Host, Port, Socket,
+                                                             Opts, User, CbInfo]),
 	{ok, SslSocket} = ssl_gen_statem:socket_control(?MODULE, Socket, [Pid, Sender], CbModule, Trackers),
         ssl_gen_statem:handshake(SslSocket, Timeout)
     catch
@@ -96,9 +97,10 @@ start_fsm(Role, Host, Port, Socket, {#{erl_dist := true},_, Trackers} = Opts,
 	  User, {CbModule, _,_, _, _} = CbInfo, 
 	  Timeout) -> 
     try 
-        {ok, Sender} = tls_sender:start([{spawn_opt, ?DIST_CNTRL_SPAWN_OPTS}]),
-	{ok, Pid} = tls_connection_sup:start_child_dist([Role, Sender, Host, Port, Socket, 
-							 Opts, User, CbInfo]), 
+        {ok, DynSup} =  tls_connection_sup:start_child_dist([]),
+        {ok, Sender} = tls_dyn_connection_sup:start_child(DynSup, sender, [[{spawn_opt, ?DIST_CNTRL_SPAWN_OPTS}]]),
+	{ok, Pid} = tls_dyn_connection_sup:start_child(DynSup, receiver, [Role, Sender, Host, Port, Socket,
+                                                                 Opts, User, CbInfo]),
 	{ok, SslSocket} = ssl_gen_statem:socket_control(?MODULE, Socket, [Pid, Sender], CbModule, Trackers),
         ssl_gen_statem:handshake(SslSocket, Timeout)
     catch
@@ -320,9 +322,6 @@ handle_info({CloseTag, Socket}, StateName,
             %% is called after all data has been deliver.
             {next_state, StateName, State#state{protocol_specific = PS#{active_n_toggle => true}}, []}
     end;
-handle_info({'EXIT', Sender, Reason}, _,
-            #state{protocol_specific = #{sender := Sender}} = State) ->
-    {stop, {shutdown, {sender_died, Reason}}, State};
 handle_info(Msg, StateName, State) ->
     ssl_gen_statem:handle_info(Msg, StateName, State).
 
diff --git a/lib/ssl/src/tls_sender.erl b/lib/ssl/src/tls_sender.erl
index 3d2cafa24c..065e5b9987 100644
--- a/lib/ssl/src/tls_sender.erl
+++ b/lib/ssl/src/tls_sender.erl
@@ -30,15 +30,30 @@
 -include("tls_handshake_1_3.hrl").
 
 %% API
--export([start/0, start/1, initialize/2, send_data/2,
-         send_post_handshake/2, send_alert/2,
-         send_and_ack_alert/2, setopts/2, renegotiate/1, peer_renegotiate/1, downgrade/2,
+-export([start_link/0,
+         start_link/1,
+         initialize/2,
+         send_data/2,
+         send_post_handshake/2,
+         send_alert/2,
+         send_and_ack_alert/2,
+         setopts/2,
+         renegotiate/1,
+         peer_renegotiate/1,
+         downgrade/2,
          update_connection_state/3,
-         dist_tls_socket/1, dist_handshake_complete/3]).
+         dist_tls_socket/1,
+         dist_handshake_complete/3]).
 
 %% gen_statem callbacks
--export([callback_mode/0, init/1, terminate/3, code_change/4]).
--export([init/3, connection/3, handshake/3, death_row/3]).
+-export([callback_mode/0,
+         init/1,
+         terminate/3,
+         code_change/4]).
+-export([init/3,
+         connection/3,
+         handshake/3,
+         death_row/3]).
 
 -record(static,
         {connection_pid,
@@ -65,10 +80,10 @@
 %%% API
 %%%===================================================================
 %%--------------------------------------------------------------------
--spec start() -> {ok, Pid :: pid()} |
+-spec start_link() -> {ok, Pid :: pid()} |
                  ignore |
                  {error, Error :: term()}.
--spec start(list()) -> {ok, Pid :: pid()} |
+-spec start_link(list()) -> {ok, Pid :: pid()} |
                        ignore |
                        {error, Error :: term()}.
 
@@ -76,10 +91,10 @@
 %%  may happen when a socket is busy (busy port) and the
 %%  same process is sending and receiving 
 %%--------------------------------------------------------------------
-start() ->
-    gen_statem:start(?MODULE, [], []).
-start(SpawnOpts) ->
-    gen_statem:start(?MODULE, [], SpawnOpts).
+start_link() ->
+    gen_statem:start_link(?MODULE, [], []).
+start_link(SpawnOpts) ->
+    gen_statem:start_link(?MODULE, [], SpawnOpts).
 
 %%--------------------------------------------------------------------
 -spec initialize(pid(), map()) -> ok. 
@@ -613,7 +628,9 @@ call(FsmPid, Event) ->
  	    {error, closed};
 	exit:{normal, _} ->
 	    {error, closed};
-	exit:{{shutdown, _},_} ->
+	exit:{shutdown,_} ->
+	    {error, closed};
+        exit:{{shutdown, _},_} ->
 	    {error, closed}
     end.
 
diff --git a/lib/ssl/src/tls_socket.erl b/lib/ssl/src/tls_socket.erl
index 0a583dbb1d..12d59c6b4c 100644
--- a/lib/ssl/src/tls_socket.erl
+++ b/lib/ssl/src/tls_socket.erl
@@ -105,15 +105,7 @@ accept(ListenSocket, #config{transport_info = {Transport,_,_,_,_} = CbInfo,
             Tracker = proplists:get_value(option_tracker, Trackers),
             {ok, EmOpts} = get_emulated_opts(Tracker),
 	    {ok, Port} = tls_socket:port(Transport, Socket),
-            {ok, Sender} = tls_sender:start(),
-            ConnArgs = [server, Sender, "localhost", Port, Socket,
-			{SslOpts, emulated_socket_options(EmOpts, #socket_options{}), Trackers}, self(), CbInfo],
-	    case tls_connection_sup:start_child(ConnArgs) of
-		{ok, Pid} ->
-		    ssl_gen_statem:socket_control(ConnectionCb, Socket, [Pid, Sender], Transport, Trackers);
-		{error, Reason} ->
-		    {error, Reason}
-	    end;
+            start_tls_server_connection(SslOpts, ConnectionCb, Transport, Port, Socket, EmOpts, Trackers, CbInfo);
 	{error, Reason} ->
 	    {error, Reason}
     end.
@@ -399,6 +391,19 @@ code_change(_OldVsn, State, _Extra) ->
 call(Pid, Msg) ->
     gen_server:call(Pid, Msg, infinity).
 
+start_tls_server_connection(SslOpts, ConnectionCb, Transport, Port, Socket, EmOpts, Trackers, CbInfo) ->    
+    try
+        {ok, DynSup} = tls_connection_sup:start_child([]),
+        {ok, Sender} = tls_dyn_connection_sup:start_child(DynSup, sender, []),
+        ConnArgs = [server, Sender, "localhost", Port, Socket,
+                    {SslOpts, emulated_socket_options(EmOpts, #socket_options{}), Trackers}, self(), CbInfo],
+        {ok, Pid} = tls_dyn_connection_sup:start_child(DynSup, receiver, ConnArgs),
+        ssl_gen_statem:socket_control(ConnectionCb, Socket, [Pid, Sender], Transport, Trackers)
+    catch
+	error:{badmatch, {error, _} = Error} ->
+            Error
+    end.
+
 split_options(Opts) ->
     split_options(Opts, emulated_options(), [], []).
 split_options([], _, SocketOpts, EmuOpts) ->
diff --git a/lib/ssl/test/tls_api_SUITE.erl b/lib/ssl/test/tls_api_SUITE.erl
index 453ccf12ee..a5f4fe3709 100644
--- a/lib/ssl/test/tls_api_SUITE.erl
+++ b/lib/ssl/test/tls_api_SUITE.erl
@@ -512,8 +512,7 @@ tls_monitor_listener(Config) when is_list(Config) ->
                                          {options, tls_monitor_listen_opts(Version, ServerOpts)}]),
     _Port2 = ssl_test_lib:inet_port(Server2),
 
-    ProlistCounts1 = supervisor:count_children(ssl_listen_tracker_sup),
-    2 = proplists:get_value(workers, ProlistCounts1),
+    2 = count_children(workers, ssl_listen_tracker_sup),
 
     Sessions = session_info(Version),
 
@@ -534,12 +533,11 @@ tls_monitor_listener(Config) when is_list(Config) ->
             ct:sleep(1000)
     end,
 
-    ProlistCounts2 = supervisor:count_children(ssl_listen_tracker_sup),
 
     Sessions1 = session_info(Version),
     true = (Sessions1 == 1),
-
-    1 = proplists:get_value(workers, ProlistCounts2).
+    
+    1 = count_children(workers, ssl_listen_tracker_sup).
 
 %%--------------------------------------------------------------------
 tls_tcp_msg() ->
@@ -826,7 +824,7 @@ transport_close_in_inital_hello(Config) when is_list(Config) ->
 
     Sup = (whereis(tls_connection_sup)),
 
-    check_connection_workers(Sup, 2),
+    check_connection_processes(Sup, 2),
 
     Acceptor ! die,
 
@@ -838,20 +836,20 @@ transport_close_in_inital_hello(Config) when is_list(Config) ->
         {'EXIT', Connector, _} ->
             ok
      end,
-    check_connection_workers(Sup, 0).
+    check_connection_processes(Sup, 0).
 
-check_connection_workers(Sup, N) ->
-    check_connection_workers(Sup, N, 5).
+check_connection_processes(Sup, N) ->
+    check_connection_processes(Sup, N, 5).
 
-check_connection_workers(Sup, N, 0) ->
-    N = proplists:get_value(workers, supervisor:count_children(Sup));
-check_connection_workers(Sup, N, M) ->
-    case proplists:get_value(workers, supervisor:count_children(Sup)) of
+check_connection_processes(Sup, N, 0) ->
+    N = count_children(supervisors, Sup);
+check_connection_processes(Sup, N, M) ->
+    case count_children(supervisors, Sup) of
         N ->
             ok;
         _ ->
             ct:sleep(500),
-            check_connection_workers(Sup, N, M-1)
+            check_connection_processes(Sup, N, M-1)
     end.
 
 %%--------------------------------------------------------------------
@@ -1100,8 +1098,9 @@ tls_monitor_client_opts(_, Opts) ->
     Opts.
 
 session_info('tlsv1.3') ->
-    ProlistCounts = supervisor:count_children(tls_server_session_ticket_sup),
-    proplists:get_value(workers, ProlistCounts);
+    count_children(workers, tls_server_session_ticket_sup);
 session_info(_) ->
-    ProlistCounts = supervisor:count_children(ssl_server_session_cache_sup),
-    proplists:get_value(workers, ProlistCounts).
+    count_children(workers, ssl_server_session_cache_sup).
+
+count_children(ChildType, SupRef) ->
+    proplists:get_value(ChildType, supervisor:count_children(SupRef)).
-- 
2.31.1

Places

File 1222-ssl-Use-significante-child-for-TLS-connections.patch of Package erlang (Revision a3c4263f873c2273804c376249a3c22f)

Places