Merge branch 'ia/inets/http-client-cancel-request/OTP-11312' into maint

* ia/inets/http-client-cancel-request/OTP-11312:
  inets: httpc - Remove dead error handling code
  inets: httpc make httpc_cancel_request/[1,2] asynchronous
  httpc: Enhanched error handling
  inets: httpc improve pipelining

6 files changed, 79 insertions, 171 deletions

diff --git a/lib/inets/doc/src/httpc.xml b/lib/inets/doc/src/httpc.xml
index d9a27e7d1e..db68cc3116 100644
--- a/lib/inets/doc/src/httpc.xml
+++ b/lib/inets/doc/src/httpc.xml
@@ -4,7 +4,7 @@
 <erlref>
   <header>
     <copyright>
-      <year>2004</year><year>2012</year>
+      <year>2004</year><year>2013</year>
       <holder>Ericsson AB. All Rights Reserved.</holder>
     </copyright>
     <legalnotice>
@@ -440,7 +440,10 @@ apply(Module, Function, [ReplyInfo | Args])
 	<v>Profile = profile() | pid() (when started <c>stand_alone</c>)</v>
       </type>
       <desc>
-        <p>Cancels an asynchronous HTTP-request. </p>
+        <p>Cancels an asynchronous HTTP-request. Note this does not guarantee
+	that the request response will not be delivered, as it is asynchronous the
+	the request may already have been completed when the cancellation arrives.
+	</p>
 
         <marker id="set_options"></marker>
       </desc>
diff --git a/lib/inets/src/http_client/httpc.erl b/lib/inets/src/http_client/httpc.erl
index 4d7023a8e9..da9bbdd1ec 100644
--- a/lib/inets/src/http_client/httpc.erl
+++ b/lib/inets/src/http_client/httpc.erl
@@ -208,16 +208,8 @@ cancel_request(RequestId) ->
 cancel_request(RequestId, Profile) 
   when is_atom(Profile) orelse is_pid(Profile) ->
     ?hcrt("cancel request", [{request_id, RequestId}, {profile, Profile}]),
-    ok = httpc_manager:cancel_request(RequestId, profile_name(Profile)), 
-    receive  
-	%% If the request was already fulfilled throw away the 
-	%% answer as the request has been canceled.
-	{http, {RequestId, _}} ->
-	    ok 
-    after 0 ->
-	    ok
-    end.
-
+    httpc_manager:cancel_request(RequestId, profile_name(Profile)).
+   
 
 %%--------------------------------------------------------------------------
 %% set_options(Options) -> ok | {error, Reason}
@@ -241,14 +233,7 @@ set_options(Options, Profile) when is_atom(Profile) orelse is_pid(Profile) ->
     ?hcrt("set options", [{options, Options}, {profile, Profile}]),
     case validate_options(Options) of
 	{ok, Opts} ->
-	    try 
-		begin
-		    httpc_manager:set_options(Opts, profile_name(Profile))
-		end
-	    catch
-		exit:{noproc, _} ->
-		    {error, inets_not_started}
-	    end;
+	    httpc_manager:set_options(Opts, profile_name(Profile));
 	{error, Reason} ->
 	    {error, Reason}
     end.
@@ -343,8 +328,6 @@ store_cookies(SetCookieHeaders, Url, Profile)
 	    ok
 	end
     catch 
-	exit:{noproc, _} ->
-	    {error, {not_started, Profile}};
 	error:{badmatch, Bad} ->
 	    {error, {parse_failed, Bad}}
     end.
diff --git a/lib/inets/src/http_client/httpc_handler.erl b/lib/inets/src/http_client/httpc_handler.erl
index 55794f57dc..80c8b2439e 100644
--- a/lib/inets/src/http_client/httpc_handler.erl
+++ b/lib/inets/src/http_client/httpc_handler.erl
@@ -32,7 +32,7 @@
          start_link/4,
          %% connect_and_send/2,
          send/2, 
-         cancel/3,
+         cancel/2,
          stream_next/1,
          info/1
         ]).
@@ -117,8 +117,8 @@ send(Request, Pid) ->
 %% Description: Cancels a request. Intended to be called by the httpc
 %% manager process.
 %%--------------------------------------------------------------------
-cancel(RequestId, Pid, From) ->
-    cast({cancel, RequestId, From}, Pid).
+cancel(RequestId, Pid) ->
+    cast({cancel, RequestId}, Pid).
 
 
 %%--------------------------------------------------------------------
@@ -400,19 +400,17 @@ handle_call(info, _, State) ->
 %% handle_keep_alive_queue/2 on the other hand will just skip the
 %% request as if it was never issued as in this case the request will
 %% not have been sent. 
-handle_cast({cancel, RequestId, From},
+handle_cast({cancel, RequestId},
             #state{request      = #request{id = RequestId} = Request,
                    profile_name = ProfileName,
                    canceled     = Canceled} = State) ->
     ?hcrv("cancel current request", [{request_id, RequestId}, 
                                      {profile,    ProfileName},
                                      {canceled,   Canceled}]),
-    httpc_manager:request_canceled(RequestId, ProfileName, From),
-    ?hcrv("canceled", []),
     {stop, normal, 
      State#state{canceled = [RequestId | Canceled],
                  request  = Request#request{from = answer_sent}}};
-handle_cast({cancel, RequestId, From},
+handle_cast({cancel, RequestId},
             #state{profile_name = ProfileName,
                    request      = #request{id = CurrId},
                    canceled     = Canceled} = State) ->
@@ -420,8 +418,6 @@ handle_cast({cancel, RequestId, From},
                      {curr_req_id, CurrId},
                      {profile, ProfileName},
                      {canceled,   Canceled}]),
-    httpc_manager:request_canceled(RequestId, ProfileName, From),
-    ?hcrv("canceled", []),
     {noreply, State#state{canceled = [RequestId | Canceled]}};
 
 handle_cast(stream_next, #state{session = Session} = State) ->
@@ -521,19 +517,12 @@ handle_info({Proto, _Socket, Data},
                 activate_once(Session),
                 {noreply, State#state{mfa = NewMFA}}
         catch
-            exit:_Exit ->
-                ?hcrd("data processing exit", [{exit, _Exit}]),
+            _:_Reason ->
+                ?hcrd("data processing exit", [{exit, _Reason}]),
                 ClientReason = {could_not_parse_as_http, Data}, 
                 ClientErrMsg = httpc_response:error(Request, ClientReason),
                 NewState     = answer_request(Request, ClientErrMsg, State),
-                {stop, normal, NewState};
-            error:_Error ->    
-                ?hcrd("data processing error", [{error, _Error}]),
-                ClientReason = {could_not_parse_as_http, Data}, 
-                ClientErrMsg = httpc_response:error(Request, ClientReason),
-                NewState     = answer_request(Request, ClientErrMsg, State),   
                 {stop, normal, NewState}
-        
         end,
     ?hcri("data processed", [{final_result, FinalResult}]),
     FinalResult;
@@ -1165,7 +1154,7 @@ handle_http_body(Body, #state{headers       = Headers,
 
 handle_response(#state{status = new} = State) ->
     ?hcrd("handle response - status = new", []),
-    handle_response(try_to_enable_pipeline_or_keep_alive(State));
+    handle_response(check_persistent(State));
 
 handle_response(#state{request      = Request,
 		       status       = Status,
@@ -1440,39 +1429,22 @@ is_keep_alive_enabled_server(_,_) ->
 is_keep_alive_connection(Headers, #session{client_close = ClientClose}) ->
     (not ((ClientClose) orelse httpc_response:is_server_closing(Headers))).
 
-try_to_enable_pipeline_or_keep_alive(
-  #state{session      = Session, 
-	 request      = #request{method = Method},
+check_persistent(
+  #state{session      = #session{type = Type} = Session, 
 	 status_line  = {Version, _, _},
 	 headers      = Headers,
-	 profile_name = ProfileName} = State) ->
-    ?hcrd("try to enable pipeline or keep-alive", 
-	  [{version, Version}, 
-	   {headers, Headers}, 
-	   {session, Session}]),
+	 profile_name = ProfileName} = State) ->    
     case is_keep_alive_enabled_server(Version, Headers) andalso 
-	  is_keep_alive_connection(Headers, Session) of
+	is_keep_alive_connection(Headers, Session) of
 	true ->
-	    case (is_pipeline_enabled_client(Session) andalso 
-		  httpc_request:is_idempotent(Method)) of
-		true ->
-		    insert_session(Session, ProfileName),
-		    State#state{status = pipeline};
-		false ->
-		    insert_session(Session, ProfileName),
-		    %% Make sure type is keep_alive in session
-		    %% as it in this case might be pipeline
-		    NewSession = Session#session{type = keep_alive}, 
-		    State#state{status  = keep_alive,
-				session = NewSession}
-	    end;
+	    mark_persistent(ProfileName, Session),
+	    State#state{status = Type};
 	false ->
 	    State#state{status = close}
     end.
 
 answer_request(#request{id = RequestId, from = From} = Request, Msg, 
-	       #state{session      = Session, 
-		      timers       = Timers, 
+	       #state{timers       = Timers, 
 		      profile_name = ProfileName} = State) -> 
     ?hcrt("answer request", [{request, Request}, {msg, Msg}]),
     httpc_response:send(From, Msg),
@@ -1482,19 +1454,14 @@ answer_request(#request{id = RequestId, from = From} = Request, Msg,
     Timer = {RequestId, TimerRef},
     cancel_timer(TimerRef, {timeout, Request#request.id}),
     httpc_manager:request_done(RequestId, ProfileName),
-    NewSession = maybe_make_session_available(ProfileName, Session), 
     Timers2 = Timers#timers{request_timers = lists:delete(Timer, 
 							  RequestTimers)}, 
     State#state{request = Request#request{from = answer_sent},
-		session = NewSession, 
 		timers  = Timers2}.
 
-maybe_make_session_available(ProfileName, 
-			     #session{available = false} = Session) ->
-    update_session(ProfileName, Session, #session.available, true),
-    Session#session{available = true};
-maybe_make_session_available(_ProfileName, Session) ->
-    Session.
+mark_persistent(ProfileName, Session) ->
+    update_session(ProfileName, Session, #session.persistent, true),
+    Session#session{persistent = true}.
     
 cancel_timers(#timers{request_timers = ReqTmrs, queue_timer = QTmr}) ->
     cancel_timer(QTmr, timeout_queue),
diff --git a/lib/inets/src/http_client/httpc_internal.hrl b/lib/inets/src/http_client/httpc_internal.hrl
index 30e2742e9d..d5b3dd2a2a 100644
--- a/lib/inets/src/http_client/httpc_internal.hrl
+++ b/lib/inets/src/http_client/httpc_internal.hrl
@@ -1,7 +1,7 @@
 %%
 %% %CopyrightBegin%
 %%
-%% Copyright Ericsson AB 2005-2012. All Rights Reserved.
+%% Copyright Ericsson AB 2005-2013. 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
@@ -143,8 +143,8 @@
 
 	  %% true | false
 	  %% This will be true, when a response has been received for 
-	  %% the first request. See type above.
-	  available = false
+	  %% the first request and the server has not closed the connection
+	  persistent = false
 	 }).
 
 
diff --git a/lib/inets/src/http_client/httpc_manager.erl b/lib/inets/src/http_client/httpc_manager.erl
index c45dcab802..a3ed371e61 100644
--- a/lib/inets/src/http_client/httpc_manager.erl
+++ b/lib/inets/src/http_client/httpc_manager.erl
@@ -1,7 +1,7 @@
 %%
 %% %CopyrightBegin%
 %% 
-%% Copyright Ericsson AB 2002-2012. All Rights Reserved.
+%% Copyright Ericsson AB 2002-2013. 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
@@ -29,7 +29,6 @@
 	 start_link/3, 
 	 request/2, 
 	 cancel_request/2,
-	 request_canceled/3,
 	 request_done/2, 
 	 retry_request/2, 
 	 redirect_request/2,
@@ -144,22 +143,7 @@ redirect_request(Request, ProfileName) ->
 %%--------------------------------------------------------------------
 
 cancel_request(RequestId, ProfileName) ->
-    call(ProfileName, {cancel_request, RequestId}).
-
-
-%%--------------------------------------------------------------------
-%% Function: request_canceled(RequestId, ProfileName) -> ok
-%%	RequestId - ref()
-%%      ProfileName = atom()
-%%
-%% Description: Confirms that a request has been canceld. Intended to
-%% be called by the httpc handler process.
-%%--------------------------------------------------------------------
-
-request_canceled(RequestId, ProfileName, From) ->
-    gen_server:reply(From, ok),
-    cast(ProfileName, {request_canceled, RequestId}).
-
+    cast(ProfileName, {cancel_request, RequestId}).
 
 %%--------------------------------------------------------------------
 %% Function: request_done(RequestId, ProfileName) -> ok
@@ -467,33 +451,13 @@ do_init(ProfileName, CookiesDir) ->
 %%--------------------------------------------------------------------
 handle_call({request, Request}, _, State) ->
     ?hcri("request", [{request, Request}]),
-    case (catch handle_request(Request, State)) of
+    case (catch handle_request(Request, State, false)) of
 	{reply, Msg, NewState} ->
 	    {reply, Msg, NewState};
 	Error ->
 	    {stop, Error, httpc_response:error(Request, Error), State}
     end;
 
-handle_call({cancel_request, RequestId}, From, 
-	    #state{handler_db = HandlerDb} = State) ->
-    ?hcri("cancel_request", [{request_id, RequestId}]),
-    case ets:lookup(HandlerDb, RequestId) of
-	[] ->
-	    %% The request has allready compleated make sure
-	    %% it is deliverd to the client process queue so
-	    %% it can be thrown away by httpc:cancel_request
-	    %% This delay is hopfully a temporary workaround.
-	    %% Note that it will not not delay the manager,
-	    %% only the client that called httpc:cancel_request
-	    timer:apply_after(?DELAY, gen_server, reply, [From, ok]),
-	    {noreply, State};
-	[{_, Pid, _}] ->
-	    httpc_handler:cancel(RequestId, Pid, From),
-	    {noreply, 
-	     State#state{cancel = 
-			 [{RequestId, Pid, From} | State#state.cancel]}}
-    end;
-
 handle_call(reset_cookies, _, #state{cookie_db = CookieDb} = State) ->
     ?hcrv("reset cookies", []),
     httpc_cookie:reset_db(CookieDb),
@@ -547,7 +511,7 @@ handle_cast({retry_or_redirect_request, {Time, Request}},
     {noreply, State};
 
 handle_cast({retry_or_redirect_request, Request}, State) ->
-    case (catch handle_request(Request, State)) of
+    case (catch handle_request(Request, State, true)) of
 	{reply, {ok, _}, NewState} ->
 	    {noreply, NewState};
 	Error  ->
@@ -555,19 +519,19 @@ handle_cast({retry_or_redirect_request, Request}, State) ->
 	    {stop, Error, State}
     end;
 
-handle_cast({request_canceled, RequestId}, State) ->
-    ?hcrv("request canceled", [{request_id, RequestId}]),
-    ets:delete(State#state.handler_db, RequestId),
-    case lists:keysearch(RequestId, 1, State#state.cancel) of
-	{value, Entry = {RequestId, _, From}} ->
-	    ?hcrt("found in cancel", [{from, From}]),
-	    {noreply, 
-	     State#state{cancel = lists:delete(Entry, State#state.cancel)}};
-	Else ->
-	    ?hcrt("not found in cancel", [{else, Else}]),
-	   {noreply, State}
+handle_cast({cancel_request, RequestId}, 
+	    #state{handler_db = HandlerDb} = State) ->
+    case ets:lookup(HandlerDb, RequestId) of
+	[] ->
+	    %% Request already compleated nothing to 
+	    %% cancel
+	    {noreply, State};
+	[{_, Pid, _}] ->
+	    httpc_handler:cancel(RequestId, Pid),
+	    ets:delete(State#state.handler_db, RequestId),
+	    {noreply, State}
     end;
-
+  
 handle_cast({request_done, RequestId}, State) ->
     ?hcrv("request done", [{request_id, RequestId}]),
     ets:delete(State#state.handler_db, RequestId),
@@ -629,22 +593,8 @@ handle_info({'EXIT', _, _}, State) ->
     %% Handled in DOWN
     {noreply, State};
 handle_info({'DOWN', _, _, Pid, _}, State) ->
-    ets:match_delete(State#state.handler_db, {'_', Pid, '_'}),
-
-    %% If there where any canceled request, handled by the
-    %% the process that now has terminated, the
-    %% cancelation can be viewed as sucessfull!
-    NewCanceldList =
-	lists:foldl(fun(Entry = {_, HandlerPid, From}, Acc)  ->
-			    case HandlerPid of
-				Pid ->
-				    gen_server:reply(From, ok),
-				    lists:delete(Entry, Acc);
-				_ ->
-				    Acc
-			    end 
-		    end, State#state.cancel, State#state.cancel),
-    {noreply, State#state{cancel = NewCanceldList}};
+    ets:match_delete(State#state.handler_db, {'_', Pid, '_'}),  
+    {noreply, State};
 handle_info(Info, State) ->
     Report = io_lib:format("Unknown message in "
 			   "httpc_manager:handle_info ~p~n", [Info]),
@@ -774,7 +724,7 @@ get_handler_info(Tab) ->
 
 handle_request(#request{settings = 
 			#http_options{version = "HTTP/0.9"}} = Request,
-	       State) ->
+	       State, _) ->
     %% Act as an HTTP/0.9 client that does not know anything
     %% about persistent connections
 
@@ -787,7 +737,7 @@ handle_request(#request{settings =
 
 handle_request(#request{settings = 
 			#http_options{version = "HTTP/1.0"}} = Request,
-	       State) ->
+	       State, _) ->
     %% Act as an HTTP/1.0 client that does not
     %% use persistent connections
 
@@ -798,13 +748,13 @@ handle_request(#request{settings =
     start_handler(NewRequest#request{headers = NewHeaders}, State),
     {reply, {ok, NewRequest#request.id}, State};
 
-handle_request(Request, State = #state{options = Options}) ->
+handle_request(Request, State = #state{options = Options}, Retry) ->
 
     NewRequest = handle_cookies(generate_request_id(Request), State),
     SessionType = session_type(Options),
     case select_session(Request#request.method,
 			Request#request.address,
-			Request#request.scheme, SessionType, State) of
+			Request#request.scheme, SessionType, State, Retry) of
 	{ok, HandlerPid} ->
 	    pipeline_or_keep_alive(NewRequest, HandlerPid, State);
 	no_connection ->
@@ -828,6 +778,7 @@ start_handler(#request{id   = Id,
 	      #state{profile_name = ProfileName, 
 		     handler_db   = HandlerDb, 
 		     options      = Options}) ->
+    ClientClose = httpc_request:is_client_closing(Request#request.headers),
     {ok, Pid} =
 	case is_inets_manager() of
 	    true ->
@@ -838,13 +789,18 @@ start_handler(#request{id   = Id,
 	end,
     HandlerInfo = {Id, Pid, From}, 
     ets:insert(HandlerDb, HandlerInfo), 
+    insert_session(#session{id = {Request#request.address, Pid},
+			    scheme = Request#request.scheme,
+			    client_close = ClientClose,
+			    type = session_type(Options)
+			   }, ProfileName),
     erlang:monitor(process, Pid).
 
 
 select_session(Method, HostPort, Scheme, SessionType, 
 	       #state{options = #options{max_pipeline_length   = MaxPipe,
 					 max_keep_alive_length = MaxKeepAlive},
-		      session_db = SessionDb}) ->
+		      session_db = SessionDb}, Retry) ->
     ?hcrd("select session", [{session_type,          SessionType},
 			     {max_pipeline_length,   MaxPipe},
 			     {max_keep_alive_length, MaxKeepAlive}]),
@@ -857,19 +813,29 @@ select_session(Method, HostPort, Scheme, SessionType,
 	    %% client_close, scheme and type specified. 
 	    %% The fields id (part of: HandlerPid) and queue_length
 	    %% specified.
-	    Pattern = #session{id           = {HostPort, '$1'},
-			       client_close = false,
-			       scheme       = Scheme,
-			       queue_length = '$2',
-			       type         = SessionType,
-			       available    = true, 
-			       _            = '_'},
+	    Pattern = case (Retry andalso SessionType == pipeline) of
+			  true ->
+			      #session{id           = {HostPort, '$1'},
+				       client_close = false,
+				       scheme       = Scheme,
+				       queue_length = '$2',
+				       type         = SessionType,
+				       persistent   = true,
+				       _            = '_'};
+			  false ->
+			      #session{id           = {HostPort, '$1'},
+				       client_close = false,
+				       scheme       = Scheme,
+				       queue_length = '$2',
+				       type         = SessionType,
+				       _            = '_'}
+		      end,
 	    %% {'_', {HostPort, '$1'}, false, Scheme, '_', '$2', SessionTyp}, 
 	    Candidates = ets:match(SessionDb, Pattern), 
 	    ?hcrd("select session", [{host_port,  HostPort}, 
 				     {scheme,     Scheme}, 
 				     {type,       SessionType}, 
-				     {candidates, Candidates}]),
+				     {candidates, Candidates}]),	    
 	    select_session(Candidates, MaxKeepAlive, MaxPipe, SessionType);
 	false ->
 	    no_connection
diff --git a/lib/inets/test/httpc_SUITE.erl b/lib/inets/test/httpc_SUITE.erl
index 0c35f284f7..818edc12ac 100644
--- a/lib/inets/test/httpc_SUITE.erl
+++ b/lib/inets/test/httpc_SUITE.erl
@@ -277,9 +277,6 @@ trace(Config) when is_list(Config) ->
 pipeline(Config) when is_list(Config) ->
     Request  = {url(group_name(Config), "/dummy.html", Config), []},
     {ok, _} = httpc:request(get, Request, [], [], pipeline),
-    
-    %% Make sure pipeline session is registerd
-    test_server:sleep(4000),
     keep_alive_requests(Request, pipeline).
 
 %%--------------------------------------------------------------------
@@ -287,9 +284,6 @@ pipeline(Config) when is_list(Config) ->
 persistent_connection(Config) when is_list(Config) ->
     Request  = {url(group_name(Config), "/dummy.html", Config), []},
     {ok, _} = httpc:request(get, Request, [], [], persistent),
-
-    %% Make sure pipeline session is registerd
-    test_server:sleep(4000),
     keep_alive_requests(Request, persistent).
 
 %%-------------------------------------------------------------------------
@@ -311,13 +305,8 @@ async(Config) when is_list(Config) ->
 
     {ok, NewRequestId} =
 	httpc:request(get, Request, [], [{sync, false}]),
-    ok = httpc:cancel_request(NewRequestId),
-    receive
-	{http, {NewRequestId, _}} ->
-	    ct:fail(http_cancel_request_failed)
-    after 3000 ->
-	    ok
-    end.
+    ok = httpc:cancel_request(NewRequestId).
+
 %%-------------------------------------------------------------------------
 save_to_file() ->
     [{doc, "Test to save the http body to a file"}].
@@ -1149,7 +1138,7 @@ receive_replys([ID|IDs]) ->
 	{http, {ID, {{_, 200, _}, [_|_], _}}} ->
 	    receive_replys(IDs);
 	{http, {Other, {{_, 200, _}, [_|_], _}}} ->
-	    ct:fail({recived_canceld_id, Other})
+	    ct:pal({recived_canceld_id, Other})
     end.
 
 %% Perform a synchronous stop