Merge branch 'ia/ssl/session-table-clean-up/OTP-9346' into maint-r14

* ia/ssl/session-table-clean-up/OTP-9346:
  The clean up of the session table now works as intended.

7 files changed, 109 insertions, 24 deletions

diff --git a/lib/ssl/src/ssl.appup.src b/lib/ssl/src/ssl.appup.src
index cf8867245b..29674f30da 100644
--- a/lib/ssl/src/ssl.appup.src
+++ b/lib/ssl/src/ssl.appup.src
@@ -1,6 +1,7 @@
 %% -*- erlang -*-
 {"%VSN%",
  [
+  {"4.1.5", [{restart_application, ssl}]},
   {"4.1.4", [{restart_application, ssl}]},
   {"4.1.3", [{restart_application, ssl}]},
   {"4.1.2", [{restart_application, ssl}]},
@@ -9,6 +10,7 @@
   {"4.0.1", [{restart_application, ssl}]}
  ], 
  [
+  {"4.1.5", [{restart_application, ssl}]},
   {"4.1.4", [{restart_application, ssl}]},
   {"4.1.3", [{restart_application, ssl}]},
   {"4.1.2", [{restart_application, ssl}]},
diff --git a/lib/ssl/src/ssl_manager.erl b/lib/ssl/src/ssl_manager.erl
index 1bbb03bdde..541ca1e918 100644
--- a/lib/ssl/src/ssl_manager.erl
+++ b/lib/ssl/src/ssl_manager.erl
@@ -265,19 +265,22 @@ handle_cast({register_session, Port, Session},
     CacheCb:update(Cache, {Port, NewSession#session.session_id}, NewSession),
     {noreply, State};
 
-handle_cast({invalidate_session, Host, Port, 
+%%% When a session is invalidated we need to wait a while before deleting
+%%% it as there might be pending connections that rightfully needs to look
+%%% up the session data but new connections should not get to use this session.
+handle_cast({invalidate_session, Host, Port,
 	     #session{session_id = ID} = Session},
 	    #state{session_cache = Cache,
 		   session_cache_cb = CacheCb} = State) ->
     CacheCb:update(Cache, {{Host, Port}, ID}, Session#session{is_resumable = false}),
-    timer:apply_after(?CLEAN_SESSION_DB, CacheCb, delete, [{{Host, Port}, ID}]),
+    timer:send_after(delay_time(), self(), {delayed_clean_session, {{Host, Port}, ID}}),
     {noreply, State};
 
 handle_cast({invalidate_session, Port, #session{session_id = ID} = Session},
 	    #state{session_cache = Cache,
 		   session_cache_cb = CacheCb} = State) ->
     CacheCb:update(Cache, {Port, ID}, Session#session{is_resumable = false}),
-    timer:apply_after(?CLEAN_SESSION_DB, CacheCb, delete, [{Port, ID}]),
+    timer:send_after(delay_time(), self(), {delayed_clean_session, {Port, ID}}),
     {noreply, State};
 
 handle_cast({recache_pem, File, LastWrite, Pid, From},
@@ -312,6 +315,12 @@ handle_info(validate_sessions, #state{session_cache_cb = CacheCb,
     start_session_validator(Cache, CacheCb, LifeTime),
     {noreply, State#state{session_validation_timer = Timer}};
 
+handle_info({delayed_clean_session, Key}, #state{session_cache = Cache,
+                   session_cache_cb = CacheCb
+                   } = State) ->
+    CacheCb:delete(Cache, Key),
+    {noreply, State};
+
 handle_info({'EXIT', _, _}, State) ->
     %% Session validator died!! Do we need to take any action?
     %% maybe error log
@@ -411,3 +420,11 @@ cache_pem_file(File, LastWrite) ->
 	[] ->
 	    call({cache_pem, File, LastWrite})
     end.
+
+delay_time() ->
+    case application:get_env(ssl, session_delay_cleanup_time) of
+	{ok, Time} when is_integer(Time) ->
+	    Time;
+	_ ->
+	   ?CLEAN_SESSION_DB
+    end.
diff --git a/lib/ssl/src/ssl_session_cache.erl b/lib/ssl/src/ssl_session_cache.erl
index 823bf7acfa..ae7c67bb98 100644
--- a/lib/ssl/src/ssl_session_cache.erl
+++ b/lib/ssl/src/ssl_session_cache.erl
@@ -1,7 +1,7 @@
 %%
 %% %CopyrightBegin%
 %%
-%% Copyright Ericsson AB 2008-2010. All Rights Reserved.
+%% Copyright Ericsson AB 2008-2011. All Rights Reserved.
 %%
 %% The contents of this file are subject to the Erlang Public License,
 %% Version 1.1, (the "License"); you may not use this file except in
@@ -36,7 +36,7 @@
 %% Description: Return table reference. Called by ssl_manager process. 
 %%--------------------------------------------------------------------
 init(_) ->
-    ets:new(cache_name(), [set, protected]).
+    ets:new(cache_name(), [named_table, set, protected]).
 
 %%--------------------------------------------------------------------
 -spec terminate(cache_ref()) -> any(). %%    
diff --git a/lib/ssl/test/ssl_basic_SUITE.erl b/lib/ssl/test/ssl_basic_SUITE.erl
index 4f0907027f..ec287ed803 100644
--- a/lib/ssl/test/ssl_basic_SUITE.erl
+++ b/lib/ssl/test/ssl_basic_SUITE.erl
@@ -1659,7 +1659,7 @@ reuse_session(Config) when is_list(Config) ->
     Server = 
 	ssl_test_lib:start_server([{node, ServerNode}, {port, 0}, 
 				   {from, self()},
-				   {mfa, {?MODULE, session_info_result, []}},
+				   {mfa, {ssl_test_lib, session_info_result, []}},
 				   {options, ServerOpts}]),
     Port = ssl_test_lib:inet_port(Server),
     Client0 =
@@ -1681,7 +1681,7 @@ reuse_session(Config) when is_list(Config) ->
     Client1 =
 	ssl_test_lib:start_client([{node, ClientNode},
 				   {port, Port}, {host, Hostname},
-				   {mfa, {?MODULE, session_info_result, []}},
+				   {mfa, {ssl_test_lib, session_info_result, []}},
 				   {from, self()},  {options, ClientOpts}]),
     receive
 	{Client1, SessionInfo} ->
@@ -1697,7 +1697,7 @@ reuse_session(Config) when is_list(Config) ->
     Client2 =
 	ssl_test_lib:start_client([{node, ClientNode},
 		      {port, Port}, {host, Hostname},
-			    {mfa, {?MODULE, session_info_result, []}},
+			    {mfa, {ssl_test_lib, session_info_result, []}},
 		      {from, self()},  {options, [{reuse_sessions, false}
 						  | ClientOpts]}]),   
     receive
@@ -1713,7 +1713,7 @@ reuse_session(Config) when is_list(Config) ->
     Server1 = 
 	ssl_test_lib:start_server([{node, ServerNode}, {port, 0}, 
 				   {from, self()},
-		      {mfa, {?MODULE, session_info_result, []}},
+		      {mfa, {ssl_test_lib, session_info_result, []}},
 		      {options, [{reuse_sessions, false} | ServerOpts]}]),
     
     Port1 = ssl_test_lib:inet_port(Server1),
@@ -1737,7 +1737,7 @@ reuse_session(Config) when is_list(Config) ->
     Client4 = 
 	ssl_test_lib:start_client([{node, ClientNode}, 
 				   {port, Port1}, {host, Hostname},
-				   {mfa, {?MODULE, session_info_result, []}},
+				   {mfa, {ssl_test_lib, session_info_result, []}},
 				   {from, self()},  {options, ClientOpts}]),
     
     receive
@@ -1756,9 +1756,6 @@ reuse_session(Config) when is_list(Config) ->
     ssl_test_lib:close(Client3),
     ssl_test_lib:close(Client4).
 
-session_info_result(Socket) ->                                            
-    ssl:session_info(Socket).
-
 %%--------------------------------------------------------------------
 reuse_session_expired(doc) -> 
     ["Test sessions is not reused when it has expired"];
@@ -1774,7 +1771,7 @@ reuse_session_expired(Config) when is_list(Config) ->
     Server = 
 	ssl_test_lib:start_server([{node, ServerNode}, {port, 0}, 
 				   {from, self()},
-		      {mfa, {?MODULE, session_info_result, []}},
+		      {mfa, {ssl_test_lib, session_info_result, []}},
 		      {options, ServerOpts}]),
     Port = ssl_test_lib:inet_port(Server),
     Client0 =
@@ -1796,7 +1793,7 @@ reuse_session_expired(Config) when is_list(Config) ->
     Client1 =
 	ssl_test_lib:start_client([{node, ClientNode}, 
 		      {port, Port}, {host, Hostname},
-		      {mfa, {?MODULE, session_info_result, []}},
+		      {mfa, {ssl_test_lib, session_info_result, []}},
 		      {from, self()},  {options, ClientOpts}]),    
     receive
 	{Client1, SessionInfo} ->
@@ -1815,7 +1812,7 @@ reuse_session_expired(Config) when is_list(Config) ->
     Client2 =
 	ssl_test_lib:start_client([{node, ClientNode}, 
 		      {port, Port}, {host, Hostname},
-				   {mfa, {?MODULE, session_info_result, []}},
+				   {mfa, {ssl_test_lib, session_info_result, []}},
 				   {from, self()},  {options, ClientOpts}]),   
     receive
 	{Client2, SessionInfo} ->
@@ -1844,7 +1841,7 @@ server_does_not_want_to_reuse_session(Config) when is_list(Config) ->
     Server = 
 	ssl_test_lib:start_server([{node, ServerNode}, {port, 0}, 
 				   {from, self()},
-		      {mfa, {?MODULE, session_info_result, []}},
+		      {mfa, {ssl_test_lib, session_info_result, []}},
 				   {options, [{reuse_session, fun(_,_,_,_) ->
 								      false
 							      end} | 
@@ -1870,7 +1867,7 @@ server_does_not_want_to_reuse_session(Config) when is_list(Config) ->
     Client1 =
 	ssl_test_lib:start_client([{node, ClientNode}, 
 		      {port, Port}, {host, Hostname},
-		      {mfa, {?MODULE, session_info_result, []}},
+		      {mfa, {ssl_test_lib, session_info_result, []}},
 		      {from, self()},  {options, ClientOpts}]),    
     receive
 	{Client1, SessionInfo} ->
@@ -3179,7 +3176,7 @@ no_reuses_session_server_restart_new_cert(Config) when is_list(Config) ->
     Server =
 	ssl_test_lib:start_server([{node, ServerNode}, {port, 0},
 				   {from, self()},
-		      {mfa, {?MODULE, session_info_result, []}},
+		      {mfa, {ssl_test_lib, session_info_result, []}},
 				   {options, ServerOpts}]),
     Port = ssl_test_lib:inet_port(Server),
     Client0 =
@@ -3207,7 +3204,7 @@ no_reuses_session_server_restart_new_cert(Config) when is_list(Config) ->
     Client1 =
 	ssl_test_lib:start_client([{node, ClientNode},
 		      {port, Port}, {host, Hostname},
-		      {mfa, {?MODULE, session_info_result, []}},
+		      {mfa, {ssl_test_lib, session_info_result, []}},
 		      {from, self()},  {options, ClientOpts}]),
     receive
 	{Client1, SessionInfo} ->
@@ -3238,7 +3235,7 @@ no_reuses_session_server_restart_new_cert_file(Config) when is_list(Config) ->
     Server =
 	ssl_test_lib:start_server([{node, ServerNode}, {port, 0},
 				   {from, self()},
-		      {mfa, {?MODULE, session_info_result, []}},
+		      {mfa, {ssl_test_lib, session_info_result, []}},
 				   {options, NewServerOpts}]),
     Port = ssl_test_lib:inet_port(Server),
     Client0 =
@@ -3268,7 +3265,7 @@ no_reuses_session_server_restart_new_cert_file(Config) when is_list(Config) ->
     Client1 =
 	ssl_test_lib:start_client([{node, ClientNode},
 		      {port, Port}, {host, Hostname},
-		      {mfa, {?MODULE, session_info_result, []}},
+		      {mfa, {ssl_test_lib, session_info_result, []}},
 				   {from, self()},  {options, ClientOpts}]),
     receive
 	{Client1, SessionInfo} ->
diff --git a/lib/ssl/test/ssl_session_cache_SUITE.erl b/lib/ssl/test/ssl_session_cache_SUITE.erl
index a43b9ab586..62d404092f 100644
--- a/lib/ssl/test/ssl_session_cache_SUITE.erl
+++ b/lib/ssl/test/ssl_session_cache_SUITE.erl
@@ -29,6 +29,7 @@
 -define(SLEEP, 500).
 -define(TIMEOUT, 60000).
 -define(LONG_TIMEOUT, 600000).
+
 -behaviour(ssl_session_cache_api).
 
 %% For the session cache tests
@@ -95,6 +96,16 @@ init_per_testcase(session_cache_process_mnesia, Config) ->
     mnesia:start(),
     init_customized_session_cache(mnesia, Config);
 
+init_per_testcase(session_cleanup, Config0) ->
+    Config = lists:keydelete(watchdog, 1, Config0),
+    Dog = test_server:timetrap(?TIMEOUT),
+    ssl:stop(),
+    application:load(ssl),
+    application:set_env(ssl, session_lifetime, 5),
+    application:set_env(ssl, session_delay_cleanup_time, ?SLEEP),
+    ssl:start(),
+    [{watchdog, Dog} | Config];
+
 init_per_testcase(_TestCase, Config0) ->
     Config = lists:keydelete(watchdog, 1, Config0),
     Dog = test_server:timetrap(?TIMEOUT),
@@ -128,6 +139,10 @@ end_per_testcase(session_cache_process_mnesia, Config) ->
     ssl:stop(),
     ssl:start(),
     end_per_testcase(default_action, Config);
+end_per_testcase(session_cleanup, Config) ->
+    application:unset_env(ssl, session_delay_cleanup_time),
+    application:unset_env(ssl, session_lifetime),
+    end_per_testcase(default_action, Config);
 end_per_testcase(_TestCase, Config) ->
     Dog = ?config(watchdog, Config),
     case Dog of
@@ -148,7 +163,8 @@ end_per_testcase(_TestCase, Config) ->
 suite() -> [{ct_hooks,[ts_install_cth]}].
 
 all() -> 
-    [session_cache_process_list,
+    [session_cleanup,
+     session_cache_process_list,
      session_cache_process_mnesia].
 
 groups() -> 
@@ -159,7 +175,57 @@ init_per_group(_GroupName, Config) ->
 
 end_per_group(_GroupName, Config) ->
     Config.
+%%--------------------------------------------------------------------
+session_cleanup(doc) ->
+    ["Test that sessions are cleand up eventually, so that the session table "
+     "does grow and grow ..."];
+session_cleanup(suite) ->
+    [];
+session_cleanup(Config)when is_list(Config) ->
+    process_flag(trap_exit, true),
+    ClientOpts = ?config(client_opts, Config),
+    ServerOpts = ?config(server_opts, Config),
+    {ClientNode, ServerNode, Hostname} = ssl_test_lib:run_where(Config),
+
+    Server =
+	ssl_test_lib:start_server([{node, ServerNode}, {port, 0},
+				   {from, self()},
+				   {mfa, {ssl_test_lib, session_info_result, []}},
+				   {options, ServerOpts}]),
+    Port = ssl_test_lib:inet_port(Server),
+    Client =
+	ssl_test_lib:start_client([{node, ClientNode},
+		      {port, Port}, {host, Hostname},
+				   {mfa, {ssl_test_lib, no_result, []}},
+		      {from, self()},  {options, ClientOpts}]),
+    SessionInfo =
+	receive
+	    {Server, Info} ->
+		Info
+	end,
+
+    %% Make sure session is registered
+    test_server:sleep(?SLEEP),
+
+    Id = proplists:get_value(session_id, SessionInfo),
+    CSession = ssl_session_cache:lookup(ssl_otp_session_cache, {{Hostname, Port}, Id}),
+    SSession = ssl_session_cache:lookup(ssl_otp_session_cache, {Port, Id}),
+
+    true = CSession =/= undefined,
+    true = SSession =/= undefined,
+
+    %% Make sure session has expired and been cleaned up
+    test_server:sleep(5000), %% Expire time
+    test_server:sleep(?SLEEP *4), %% Clean up delay
+
+    undefined = ssl_session_cache:lookup(ssl_otp_session_cache, {{Hostname, Port}, Id}),
+    undefined = ssl_session_cache:lookup(ssl_otp_session_cache, {Port, Id}),
+
+    process_flag(trap_exit, false),
+    ssl_test_lib:close(Server),
+    ssl_test_lib:close(Client).
 
+%%--------------------------------------------------------------------
 session_cache_process_list(doc) ->
     ["Test reuse of sessions (short handshake)"];
 
diff --git a/lib/ssl/test/ssl_test_lib.erl b/lib/ssl/test/ssl_test_lib.erl
index 40bbdf1dbd..b7916b96eb 100644
--- a/lib/ssl/test/ssl_test_lib.erl
+++ b/lib/ssl/test/ssl_test_lib.erl
@@ -670,3 +670,6 @@ cipher_result(Socket, Result) ->
 	Other ->
 	    {unexpected, Other}
     end.
+
+session_info_result(Socket) ->
+    ssl:session_info(Socket).
diff --git a/lib/ssl/vsn.mk b/lib/ssl/vsn.mk
index 0e80e42637..64a7603c44 100644
--- a/lib/ssl/vsn.mk
+++ b/lib/ssl/vsn.mk
@@ -1 +1 @@
-SSL_VSN = 4.1.5
+SSL_VSN = 4.1.5.1