Skip to content

Commit

Permalink
OTP-19221 httpc timeout on handle_answer
Browse files Browse the repository at this point in the history
  • Loading branch information
Whaileee committed Sep 24, 2024
1 parent 299be8d commit c47436b
Show file tree
Hide file tree
Showing 5 changed files with 127 additions and 68 deletions.
56 changes: 43 additions & 13 deletions lib/inets/src/http_client/httpc.erl
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,9 @@ Options details:
- **`t:pid/0`** - Messages are sent to this process in the format
`{http, ReplyInfo}`.

- **`alias/0`** - Messages are sent to this special reference in the format
`{http, ReplyInfo}`.

- **`function/1`** - Information is delivered to the receiver through calls to
the provided fun `Receiver(ReplyInfo)`.

Expand Down Expand Up @@ -1205,14 +1208,15 @@ handle_request(Method, Url,
started = Started,
unix_socket = UnixSocket,
ipv6_host_with_brackets = BracketedHost,
request_options = Options},
case httpc_manager:request(Request, profile_name(Profile)) of
{ok, RequestId} ->
handle_answer(RequestId, Sync, Options);
{error, Reason} ->
{error, Reason}
end
end
request_options = Options},
case httpc_manager:request(Request, profile_name(Profile)) of
{ok, RequestId} ->
handle_answer(RequestId, Receiver, Sync, Options,
element(#http_options.timeout, HTTPOptions));
{error, Reason} ->
{error, Reason}
end
end
catch
error:{noproc, _} ->
{error, {not_started, Profile}};
Expand Down Expand Up @@ -1264,20 +1268,41 @@ mk_chunkify_fun(ProcessBody) ->
end.
handle_answer(RequestId, false, _) ->
handle_answer(RequestId, _, false, _, _) ->
{ok, RequestId};
handle_answer(RequestId, true, Options) ->
handle_answer(RequestId, ClientAlias, true, Options, Timeout) ->
receive
{http, {RequestId, {ok, saved_to_file}}} ->
true = unalias(ClientAlias),
{ok, saved_to_file};
{http, {RequestId, {error, Reason}}} ->
true = unalias(ClientAlias),
{error, Reason};
{http, {RequestId, {ok, {StatusLine,Headers,BinBody}}}} ->
{http, {RequestId, {ok, {StatusLine, Headers, BinBody}}}} ->
true = unalias(ClientAlias),
Body = maybe_format_body(BinBody, Options),
{ok, {StatusLine, Headers, Body}};
{http, {RequestId, {ok, {StatusCode,BinBody}}}} ->
{http, {RequestId, {ok, {StatusCode, BinBody}}}} ->
true = unalias(ClientAlias),
Body = maybe_format_body(BinBody, Options),
{ok, {StatusCode, Body}}
after Timeout ->
cancel_request(RequestId),
true = unalias(ClientAlias),
receive
{http, {RequestId, {ok, saved_to_file}}} ->
{ok, saved_to_file};
{http, {RequestId, {error, Reason}}} ->
{error, Reason};
{http, {RequestId, {ok, {StatusLine, Headers, BinBody}}}} ->
Body = maybe_format_body(BinBody, Options),
{ok, {StatusLine, Headers, Body}};
{http, {RequestId, {ok, {StatusCode, BinBody}}}} ->
Body = maybe_format_body(BinBody, Options),
{ok, {StatusCode, Body}}
after 0 ->
{error, timeout}
end
end.
maybe_format_body(BinBody, Options) ->
Expand Down Expand Up @@ -1474,6 +1499,8 @@ request_options_defaults() ->
ok;
(Value) when is_function(Value, 1) ->
ok;
(Value) when is_reference(Value) ->
ok;
(_) ->
error
end,
Expand All @@ -1495,7 +1522,7 @@ request_options_defaults() ->
{body_format, string, VerifyBodyFormat},
{full_result, true, VerifyFullResult},
{headers_as_is, false, VerifyHeaderAsIs},
{receiver, self(), VerifyReceiver},
{receiver, alias(), VerifyReceiver},
{socket_opts, undefined, VerifySocketOpts},
{ipv6_host_with_brackets, false, VerifyBrackets}
].
Expand Down Expand Up @@ -1549,6 +1576,7 @@ request_options([{Key, DefaultVal, Verify} | Defaults], Options, Acc) ->
BodyFormat :: string | binary,
SocketOpt :: term(),
Receiver :: pid()
| reference()
| fun((term()) -> term())
| { ReceiverModule::atom()
, ReceiverFunction::atom()
Expand All @@ -1559,6 +1587,8 @@ request_options_sanity_check(Opts) ->
case proplists:get_value(receiver, Opts) of
Pid when is_pid(Pid) andalso (Pid =:= self()) ->
ok;
Reference when is_reference(Reference) ->
ok;
BadReceiver ->
throw({error, {bad_options_combo,
[{sync, true}, {receiver, BadReceiver}]}})
Expand Down
15 changes: 7 additions & 8 deletions lib/inets/src/http_client/httpc_handler.erl
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,6 @@ do_handle_info({Proto, _Socket, Data},
when (Proto =:= tcp) orelse
(Proto =:= ssl) orelse
(Proto =:= httpc_handler) ->

try Module:Function([Data | Args]) of
{ok, Result} ->
handle_http_msg(Result, State);
Expand Down Expand Up @@ -1320,9 +1319,9 @@ handle_server_closing(State = #state{headers = Headers}) ->
end.

answer_request(#request{id = RequestId, from = From, request_options = Options} = Request, Msg,
#state{session = Session,
timers = Timers,
profile_name = ProfileName} = State) ->
#state{session = Session,
timers = Timers,
profile_name = ProfileName} = State) ->
Answer = format_answer(Msg, Options),
httpc_response:send(From, Answer),
RequestTimers = Timers#timers.request_timers,
Expand Down Expand Up @@ -1718,10 +1717,10 @@ format_address({[$[|T], Port}) ->
format_address(HostPort) ->
HostPort.

format_answer(Res0, Options) ->
format_answer(Res, Options) ->
FullResult = proplists:get_value(full_result, Options, true),
Sync = proplists:get_value(sync, Options, true),
do_format_answer(Res0, FullResult, Sync).
do_format_answer(Res, FullResult, Sync).
do_format_answer({Ref, StatusLine}, _, Sync) when is_atom(StatusLine) ->
case Sync of
true ->
Expand All @@ -1742,9 +1741,9 @@ do_format_answer({Ref, {StatusLine, Headers, BinBody}}, true, Sync) ->
{Ref, {ok, {StatusLine, Headers, BinBody}}};
_ ->
{Ref, {StatusLine, Headers, BinBody}}
end;
end;
do_format_answer({Ref, {StatusLine, _, BinBody}}, false, Sync) ->
{_, Status, _} = StatusLine,
{_, Status, _} = StatusLine,
case Sync of
true ->
{Ref, {ok, {Status, BinBody}}};
Expand Down
50 changes: 25 additions & 25 deletions lib/inets/src/http_client/httpc_request.erl
Original file line number Diff line number Diff line change
Expand Up @@ -56,32 +56,32 @@ send(SendAddr, #session{socket = Socket, socket_type = SocketType}, Request) ->
send(SendAddr, Socket, SocketType, Request).

send(SendAddr, Socket, SocketType,
#request{method = Method,
path = Path,
pquery = Query,
headers = Headers,
content = Content,
address = Address,
abs_uri = AbsUri,
headers_as_is = HeadersAsIs,
settings = HttpOptions,
userinfo = UserInfo,
request_options = Options}) ->
#request{method = Method,
path = Path,
pquery = Query,
headers = Headers,
content = Content,
address = Address,
abs_uri = AbsUri,
headers_as_is = HeadersAsIs,
settings = HttpOptions,
userinfo = UserInfo,
request_options = Options}) ->

?hcrt("send",
[{send_addr, SendAddr},
{socket, Socket},
{method, Method},
{path, Path},
{pquery, Query},
{headers, Headers},
{content, Content},
{address, Address},
{abs_uri, AbsUri},
{headers_as_is, HeadersAsIs},
{settings, HttpOptions},
{userinfo, UserInfo},
{request_options, Options}]),
[{send_addr, SendAddr},
{socket, Socket},
{method, Method},
{path, Path},
{pquery, Query},
{headers, Headers},
{content, Content},
{address, Address},
{abs_uri, AbsUri},
{headers_as_is, HeadersAsIs},
{settings, HttpOptions},
{userinfo, UserInfo},
{request_options, Options}]),

TmpHdrs = handle_user_info(UserInfo, Headers),

Expand Down
2 changes: 1 addition & 1 deletion lib/inets/src/http_client/httpc_response.erl
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ result(Response = {{_,Code,_}, _, _}, Request) when (Code div 100) =:= 5 ->
result(Response, Request) ->
transparent(Response, Request).

send(Receiver, Msg) when is_pid(Receiver) ->
send(Receiver, Msg) when is_pid(Receiver); is_reference(Receiver) ->
Receiver ! {http, Msg};
send(Receiver, Msg) when is_function(Receiver) ->
(catch Receiver(Msg));
Expand Down
72 changes: 51 additions & 21 deletions lib/inets/test/httpc_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,9 @@ init_per_testcase(Name, Config) when Name == pipeline; Name == persistent_connec
{max_pipeline_length, 3} | GivenOptions], Name),

[{profile, Name} | Config];
init_per_testcase(async, Config) ->
{ok,Pid} = inets:start(httpc, [{profile, async}], stand_alone),
[{httpc_pid, Pid} | Config];
init_per_testcase(Case, Config) ->
{ok, _Pid} = inets:start(httpc, [{profile, Case}]),
GivenOptions = proplists:get_value(httpc_options, Config, []),
Expand All @@ -367,7 +370,9 @@ end_per_testcase(Case, Config)
ok
end,
inets:stop(httpc, ?config(profile, Config));

end_per_testcase(async, Config) ->
Pid = proplists:get_value(httpc_pid, Config),
inets:stop(httpc, Pid);
end_per_testcase(_Case, Config) ->
inets:stop(httpc, ?config(profile, Config)).

Expand Down Expand Up @@ -559,21 +564,22 @@ async() ->
[{doc, "Test an asynchrony http request."}].
async(Config) when is_list(Config) ->
Request = {url(group_name(Config), "/dummy.html", Config), []},

HttpcPid = proplists:get_value(httpc_pid, Config),
{ok, RequestId} =
httpc:request(get, Request, [?SSL_NO_VERIFY], [{sync, false}], ?profile(Config)),
httpc:request(get, Request, [?SSL_NO_VERIFY], [{sync, false}], ?profile(Config)),
Body =
receive
{http, {RequestId, {{_, 200, _}, _, BinBody}}} ->
BinBody;
{http, Msg} ->
ct:fail(Msg)
end,
receive
{http, {RequestId, {{_, 200, _}, _, BinBody}}} ->
BinBody;
{http, Msg} ->
ct:fail(Msg)
end,
inets_test_lib:check_body(binary_to_list(Body)),

%% Check full result false option for async request
{ok, RequestId2} =
httpc:request(get, Request, [?SSL_NO_VERIFY], [{sync, false},
{full_result, false}], ?profile(Config)),
{full_result, false}]),
Body2 =
receive
{http, {RequestId2, {200, BinBody2}}} ->
Expand All @@ -582,6 +588,19 @@ async(Config) when is_list(Config) ->
ct:fail(Msg2)
end,
inets_test_lib:check_body(binary_to_list(Body2)),

%% Check receiver alias() option for async request with stand_alone httpc
{ok, RequestId3} =
httpc:request(get, Request, [?SSL_NO_VERIFY], [{sync, false},
{receiver, alias()}], HttpcPid),
Body3 =
receive
{http, {RequestId3, {{_, 200, _}, _, BinBody3}}} ->
BinBody3;
{http, Msg3} ->
ct:fail(Msg3)
end,
inets_test_lib:check_body(binary_to_list(Body3)),
{ok, NewRequestId} =
httpc:request(get, Request, [?SSL_NO_VERIFY], [{sync, false}]),
ok = httpc:cancel_request(NewRequestId).
Expand Down Expand Up @@ -1703,19 +1722,30 @@ timeout_memory_leak(Config) when is_list(Config) ->
{ok, Host} = inet:gethostname(),
Request = {?URL_START ++ Host ++ ":" ++ integer_to_list(Port) ++ "/dummy.html", []},
Profile = ?config(profile, Config),
WaitForCancelRequestToFinish =
fun F(Handlers = [_ | _]) when is_list(Handlers) -> ct:fail({unexpected_handlers, Handlers});
F(Handlers) when is_list(Handlers) -> ok;
F(N) when is_integer(N) ->
Info = httpc:info(Profile),
ct:log("Info: ~p", [Info]),
{value, {handlers, Handlers}} =
lists:keysearch(handlers, 1, Info),
case Handlers of
[] ->
ok;
_ ->
ct:sleep(1)
end,
case N of
0 ->
F(Handlers);
_ ->
F(N-1)
end
end,
case httpc:request(get, Request, [{connect_timeout, 500}, {timeout, 1}], [{sync, true}], Profile) of
{error, timeout} ->
%% And now we check the size of the handler db
Info = httpc:info(Profile),
ct:log("Info: ~p", [Info]),
{value, {handlers, Handlers}} =
lists:keysearch(handlers, 1, Info),
case Handlers of
[] ->
ok;
_ ->
ct:fail({unexpected_handlers, Handlers})
end;
WaitForCancelRequestToFinish(5);
Unexpected ->
ct:fail({unexpected, Unexpected})
end.
Expand Down

0 comments on commit c47436b

Please sign in to comment.