Fixed httpc manager crash

httpc manager crashes.When a request results in a retry, the request
id will be "reused" in the previous implementation a race condition
could occur causing the manager to crash. This is now avoided by using
proc_lib:init_ack and gen_server:enter_loop to allow more advanced
initialization of httpc_handlers without blocking the httpc_manger
and eliminating extra processes that can cause race conditions.

1 files changed, 79 insertions, 177 deletions

diff --git a/lib/inets/src/http_client/httpc_handler.erl b/lib/inets/src/http_client/httpc_handler.erl
index 5e22400fe0..1f0e012e7e 100644
--- a/lib/inets/src/http_client/httpc_handler.erl
+++ b/lib/inets/src/http_client/httpc_handler.erl
@@ -1,7 +1,7 @@
 %%
 %% %CopyrightBegin%
 %% 
-%% Copyright Ericsson AB 2002-2010. All Rights Reserved.
+%% Copyright Ericsson AB 2002-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
@@ -29,10 +29,10 @@
 %%--------------------------------------------------------------------
 %% Internal Application API
 -export([
-	 start_link/2, 
-	 connect_and_send/2, 
+	 start_link/4,
+	 %% connect_and_send/2,
 	 send/2, 
-	 cancel/2, 
+	 cancel/3,
 	 stream/3, 
 	 stream_next/1,
 	 info/1
@@ -51,7 +51,7 @@
 -record(state, 
 	{
 	  request,                   % #request{}
-	  session,                   % #tcp_session{} 
+	  session,                   % #session{}
 	  status_line,               % {Version, StatusCode, ReasonPharse}
 	  headers,                   % #http_response_h{}
 	  body,                      % binary()
@@ -94,13 +94,9 @@
 %%--------------------------------------------------------------------
 %%--------------------------------------------------------------------
 
-start_link(Options, ProfileName) ->
-    Args = [Options, ProfileName], 
-    gen_server:start_link(?MODULE, Args, []).
-
-connect_and_send(Request, HandlerPid) ->
-    call({connect_and_send, Request}, HandlerPid).
-
+start_link(Parent, Request, Options, ProfileName) ->
+    {ok, proc_lib:start_link(?MODULE, init, [[Parent, Request, Options,
+					      ProfileName]])}.
 
 %%--------------------------------------------------------------------
 %% Function: send(Request, Pid) -> ok 
@@ -122,8 +118,8 @@ send(Request, Pid) ->
 %% Description: Cancels a request. Intended to be called by the httpc
 %% manager process.
 %%--------------------------------------------------------------------
-cancel(RequestId, Pid) ->
-    cast({cancel, RequestId}, Pid).
+cancel(RequestId, Pid, From) ->
+    cast({cancel, RequestId, From}, Pid).
 
 
 %%--------------------------------------------------------------------
@@ -229,16 +225,27 @@ stream(BodyPart, Request,_) -> % only 200 and 206 responses can be streamed
 %% but we do not want that so errors will be handled by the process
 %% sending an init_error message to itself.
 %%--------------------------------------------------------------------
-init([Options, ProfileName]) ->
-    ?hcrv("init - starting", [{options, Options}, {profile, ProfileName}]),
+init([Parent, Request, Options, ProfileName]) ->
     process_flag(trap_exit, true),
-    handle_verbose(Options#options.verbose),
-    State = #state{status       = undefined,
-		   options      = Options, 
-		   profile_name = ProfileName},
-    ?hcrd("init - started", []),
-    {ok, State}.
 
+    %% Do not let initial tcp-connection block the manager-process
+    proc_lib:init_ack(Parent, self()),
+    handle_verbose(Options#options.verbose),
+    Address = handle_proxy(Request#request.address, Options#options.proxy),
+    {ok, State} =
+	case {Address /= Request#request.address, Request#request.scheme} of
+	    {true, https} ->
+		Error = https_through_proxy_is_not_currently_supported,
+		self() ! {init_error,
+			  Error, httpc_response:error(Request, Error)},
+		{ok, #state{request = Request, options = Options,
+			    status = ssl_tunnel}};
+	    {_, _} ->
+		connect_and_send_first_request(Address, Request,
+				   #state{options = Options,
+					  profile_name = ProfileName})
+	end,
+    gen_server:enter_loop(?MODULE, [], State).
 
 %%--------------------------------------------------------------------
 %% Function: handle_call(Request, From, State) -> {reply, Reply, State} |
@@ -249,41 +256,6 @@ init([Options, ProfileName]) ->
 %%          {stop, Reason, State}            (terminate/2 is called)
 %% Description: Handling call messages
 %%--------------------------------------------------------------------
-
-
-%% This is the first request, the reason the proc was started
-handle_call({connect_and_send, #request{address = Address0,
-					scheme  = Scheme} = Request}, 
-	    _From, 
-	    #state{options = #options{proxy = Proxy}, 
-		   status  = undefined,
-		   session = undefined} = State) ->
-    ?hcrv("connect and send", [{address0, Address0}, {proxy, Proxy}]),
-    Address = handle_proxy(Address0, Proxy),
-    if
-	((Address =/= Address0) andalso (Scheme  =:= https)) ->
-	    %% This is what we should do if and when ssl supports 
-	    %% "socket upgrading"
-	    %%send_ssl_tunnel_request(Address, Request,
-	    %%		    #state{options = Options,
-	    %%		   status = ssl_tunnel});
-	    Reason = {failed_connecting, 
-		      https_through_proxy_is_not_currently_supported},
-	    %% Send a reply to the original caller
-	    ErrorResponse = httpc_response:error(Request, Reason), 
-	    httpc_response:send(Request#request.from, ErrorResponse),
-	    %% Reply to the manager
-	    ErrorReply    = {error, Reason},
-	    {stop, normal, ErrorReply, State};
-	true ->
-	    case connect_and_send_first_request(Address, Request, State) of
-		{ok, NewState} ->
-		    {reply, ok, NewState};
-		{stop, Error, NewState} ->
-		    {stop, normal, Error, NewState}
-	    end
-    end;
-	
 handle_call(#request{address = Addr} = Request, _, 
 	    #state{status  = Status,
 		   session = #session{type = pipeline} = Session,
@@ -445,25 +417,27 @@ 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}, 
+handle_cast({cancel, RequestId, From},
 	    #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),
+    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}, 
+handle_cast({cancel, RequestId, From},
 	    #state{profile_name = ProfileName,
+		   request      = #request{id = CurrId},
 		   canceled     = Canceled} = State) ->
-    ?hcrv("cancel", [{request_id, RequestId}, 
+    ?hcrv("cancel", [{request_id, RequestId},
+		     {curr_req_id, CurrId},
 		     {profile, ProfileName},
 		     {canceled,   Canceled}]),
-    httpc_manager:request_canceled(RequestId, ProfileName),
+    httpc_manager:request_canceled(RequestId, ProfileName, From),
     ?hcrv("canceled", []),
     {noreply, State#state{canceled = [RequestId | Canceled]}};
 
@@ -872,62 +846,55 @@ connect(SocketType, ToAddress,
 	    Opts3 = [IpFamily | Opts2], 
 	    http_transport:connect(SocketType, ToAddress, Opts3, Timeout)
     end.
-		
-connect_and_send_first_request(Address, 
-			       #request{settings    = Settings,
-					headers     = Headers,
-					address     = OrigAddress,
-					scheme      = Scheme} = Request, 
-			       #state{options = Options} = State) ->
-
-    ?hcrd("connect", 
-	  [{address, Address}, {request, Request}, {options, Options}]),
 
+connect_and_send_first_request(Address, Request, #state{options = Options} = State) ->
     SocketType  = socket_type(Request),
-    ConnTimeout = Settings#http_options.connect_timeout,
+    ConnTimeout = (Request#request.settings)#http_options.connect_timeout,
+    ?hcri("connect",
+	  [{address, Address}, {request, Request}, {options, Options}]),
     case connect(SocketType, Address, Options, ConnTimeout) of
 	{ok, Socket} ->
-	    Session = #session{id          = {OrigAddress, self()},
-			       scheme      = Scheme,
-			       socket      = Socket,
-			       socket_type = SocketType}, 
-	    ?hcrd("connected - now send first request", [{socket, Socket}]),
+	    ClientClose =
+			httpc_request:is_client_closing(
+			  Request#request.headers),
+	    SessionType = httpc_manager:session_type(Options),
+	    SocketType  = socket_type(Request),
+	    Session = #session{id = {Request#request.address, self()},
+			       scheme = Request#request.scheme,
+			       socket = Socket,
+			       socket_type = SocketType,
+			       client_close = ClientClose,
+			       type = SessionType},
+	    ?hcri("connected - now send first request", [{socket, Socket}]),
+
 	    case httpc_request:send(Address, Session, Request) of
 		ok ->
-		    ?hcrd("first request sent", []),
-		    ClientClose = 
-			httpc_request:is_client_closing(Headers),
-		    SessionType = httpc_manager:session_type(Options),
-		    Session2 =
-			Session#session{client_close = ClientClose,
-					type         = SessionType},
-		    TmpState = 
-			State#state{request     = Request, 
-				    session     = Session2, 
-				    mfa         = init_mfa(Request, State),
-				    status_line = init_status_line(Request),
-				    headers     = undefined,
-				    body        = undefined,
-				    status      = new},
-		    ?hcrt("activate socket", []),
-		    activate_once(Session),
+		    ?hcri("first request sent", []),
+		    TmpState = State#state{request = Request,
+					   session = Session,
+					   mfa = init_mfa(Request, State),
+					   status_line =
+					   init_status_line(Request),
+					   headers = undefined,
+					   body = undefined,
+					   status = new},
+		    http_transport:setopts(SocketType,
+					   Socket, [{active, once}]),
 		    NewState = activate_request_timeout(TmpState),
 		    {ok, NewState};
-
-		{error, Reason} = Error -> 
-		    ?hcrv("failed sending request", [{reason, Reason}]),
-		    {stop, Error, 
-		     State#state{session = {send_failed, Reason}, 
-				 request = Request}}
+		{error, Reason} ->
+		    self() ! {init_error, error_sending,
+			      httpc_response:error(Request, Reason)},
+		    {ok, State#state{request = Request,
+				     session =
+					 #session{socket = Socket}}}
 	    end;
-
-	{error, Reason} = Error -> 
-	    ?hcri("connect failed", [{reason, Reason}]),
-	    {stop, Error, State#state{session = {connect_failed, Reason},
-				       request = Request}}
+	{error, Reason} ->
+	    self() ! {init_error, error_connecting,
+		      httpc_response:error(Request, Reason)},
+	    {ok, State#state{request = Request}}
     end.
 
-
 handler_info(#state{request     = Request, 
 		    session     = Session, 
 		    status_line = _StatusLine, 
@@ -1167,12 +1134,12 @@ handle_response(#state{request      = Request,
 	{ok, Msg, Data} ->
 	    ?hcrd("handle response - ok", []),
 	    end_stream(StatusLine, Request),
-	    NewState = answer_request(Request, Msg, State),
+	    NewState = maybe_send_answer(Request, Msg, State),
 	    handle_queue(NewState, Data); 
 	{stop, Msg} ->
 	    ?hcrd("handle response - stop", [{msg, Msg}]),
 	    end_stream(StatusLine, Request),
-	    NewState = answer_request(Request, Msg, State),
+	    NewState = maybe_send_answer(Request, Msg, State),
 	    {stop, normal, NewState}
     end.
 
@@ -1242,7 +1209,8 @@ handle_pipeline(#state{status       = pipeline,
 		    %% See comment for handle_cast({cancel, RequestId})
 		    {stop, normal, 
 		     State#state{request = 
-				 NextRequest#request{from = answer_sent}}};
+				 NextRequest#request{from = answer_sent},
+				 pipeline = Pipeline}};
 		false ->
 		    ?hcrv("next request", [{request, NextRequest}]),
 		    NewSession = 
@@ -1443,6 +1411,7 @@ 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),
+
     State#state{request = Request#request{from = answer_sent},
 		timers = 
 		Timers#timers{request_timers =
@@ -1662,67 +1631,6 @@ handle_verbose(_) ->
     ok.    
 
 
-%%% Normaly I do not comment out code, I throw it away. But this might
-%%% actually be used one day if ssl is improved.
-%% send_ssl_tunnel_request(Address, Request = #request{address = {Host, Port}}, 
-%% 			State) ->
-%%     %% A ssl tunnel request is a special http request that looks like
-%%     %% CONNECT host:port HTTP/1.1
-%%     SslTunnelRequest = #request{method = connect, scheme = http,
-%% 				headers = 
-%% 				#http_request_h{
-%% 				  host = Host, 
-%% 				  address = Address, 
-%% 				  path = Host ++ ":",
-%% 				  pquery = integer_to_list(Port),
-%% 				  other = [{ "Proxy-Connection", "keep-alive"}]},
-%%     Ipv6 = (State#state.options)#options.ipv6,
-%%     SocketType = socket_type(SslTunnelRequest),
-%%     case http_transport:connect(SocketType, 
-%%                                 SslTunnelRequest#request.address, Ipv6) of
-%% 	{ok, Socket} ->
-%% 	    case httpc_request:send(Address, SslTunnelRequest, Socket) of
-%% 		ok ->
-%% 		    Session = #tcp_session{id = 
-%% 					   {SslTunnelRequest#request.address,
-%% 					    self()},
-%% 					   scheme = 
-%% 					   SslTunnelRequest#request.scheme,
-%% 					   socket = Socket},
-%% 		    NewState = State#state{mfa = 
-%% 					   {httpc_response, parse,
-%% 					    [State#state.max_header_size]},
-%% 					   request = Request,
-%% 					   session = Session},
-%% 		    http_transport:setopts(socket_type(
-%%                                           SslTunnelRequest#request.scheme), 
-%% 					   Socket, 
-%% 					   [{active, once}]),
-%% 		    {ok, NewState};
-%% 		{error, Reason} -> 
-%% 		    self() ! {init_error, error_sending, 
-%% 			      httpc_response:error(Request, Reason)},
-%% 		    {ok, State#state{request = Request,
-%% 				     session = #tcp_session{socket = 
-%% 							    Socket}}}
-%% 	    end;
-%% 	{error, Reason} ->
-%% 	    self() ! {init_error, error_connecting, 
-%% 		      httpc_response:error(Request, Reason)},
-%% 	    {ok, State#state{request = Request}}
-%%     end.
-
-%% d(F) ->
-%%    d(F, []).
-
-%% d(F, A) -> 
-%%     d(get(dbg), F, A).
-
-%% d(true, F, A) ->
-%%     io:format(user, "~w:~w:" ++ F ++ "~n", [self(), ?MODULE | A]);
-%% d(_, _, _) ->
-%%     ok.
-
 
 send_raw(#session{socket = Socket, socket_type = SocketType}, 
 	 {ProcessBody, Acc}) when is_function(ProcessBody, 1) ->
@@ -1756,11 +1664,5 @@ call(Msg, Pid, Timeout) ->
 cast(Msg, Pid) ->
     gen_server:cast(Pid, Msg).
 
-
-%% to(To, Start) when is_integer(Start) andalso (Start >= 0) ->
-%%     http_util:timeout(To, Start);
-%% to(To, _Start) ->
-%%     http_util:timeout(To, t()).
-
 t() ->
     http_util:timestamp().