The clean up of the session table now works as intended.

In ssl-4.1.5 temporary clean-up processes would crash resulting in
that the session table would not be cleaned up (e.i. using more and
more memory) and error reports would be printed, but connections would
not be affected.

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..8286201df4 100644
--- a/lib/ssl/vsn.mk
+++ b/lib/ssl/vsn.mk
@@ -1 +1 @@
-SSL_VSN = 4.1.5
+SSL_VSN = 4.1.6