From 2c27a2e2a57d37df1f33f481dc0edbfd2cc83db3 Mon Sep 17 00:00:00 2001 From: Micael Karlberg Date: Tue, 22 May 2012 14:53:45 +0200 Subject: [inets/httpc] Cancel request does not work Cancel request does not work due to incorrect handler table creation (wrong keypos). OTP-10092 --- lib/inets/src/http_client/httpc_manager.erl | 105 +++++++++-------- lib/inets/test/httpc_SUITE.erl | 177 +++++++++++++++------------- lib/inets/test/inets_test_lib.erl | 4 + 3 files changed, 154 insertions(+), 132 deletions(-) (limited to 'lib/inets') diff --git a/lib/inets/src/http_client/httpc_manager.erl b/lib/inets/src/http_client/httpc_manager.erl index b225b43214..771968e169 100644 --- a/lib/inets/src/http_client/httpc_manager.erl +++ b/lib/inets/src/http_client/httpc_manager.erl @@ -59,17 +59,9 @@ options = #options{} }). --record(handler_info, - { - id, % Id of the request: request_id() - starter, % Pid of the handler starter process (temp): pid() - handler, % Pid of the handler process: pid() - from, % From for the request: from() - state % State of the handler: initiating | started | operational | canceled - }). - -define(DELAY, 500). + %%==================================================================== %% Internal Application API %%==================================================================== @@ -379,8 +371,7 @@ do_init(ProfileName, CookiesDir) -> %% Create handler db ?hcrt("create handler/request db", []), HandlerDbName = handler_db_name(ProfileName), - ets:new(HandlerDbName, - [protected, set, named_table, {keypos, #handler_info.id}]), + ets:new(HandlerDbName, [protected, set, named_table, {keypos, 1}]), %% Cookie DB ?hcrt("create cookie db", []), @@ -414,9 +405,10 @@ handle_call({request, Request}, _, State) -> {stop, Error, httpc_response:error(Request, Error), State} end; -handle_call({cancel_request, RequestId}, From, State) -> +handle_call({cancel_request, RequestId}, From, + #state{handler_db = HandlerDb} = State) -> ?hcri("cancel_request", [{request_id, RequestId}]), - case ets:lookup(State#state.handler_db, RequestId) of + case ets:lookup(HandlerDb, RequestId) of [] -> %% The request has allready compleated make sure %% it is deliverd to the client process queue so @@ -428,9 +420,9 @@ handle_call({cancel_request, RequestId}, From, State) -> {noreply, State}; [{_, Pid, _}] -> httpc_handler:cancel(RequestId, Pid, From), - {noreply, State#state{cancel = - [{RequestId, Pid, From} | - State#state.cancel]}} + {noreply, + State#state{cancel = + [{RequestId, Pid, From} | State#state.cancel]}} end; handle_call(reset_cookies, _, #state{cookie_db = CookieDb} = State) -> @@ -645,7 +637,7 @@ code_change(_, code_change(_, State, _) -> {ok, State}. -%% This function is to catch everything that calls through the cracks... +%% This function is used to catch everything that falls through the cracks... update_session_table(SessionDB, Transform) -> ets:safe_fixtable(SessionDB, true), update_session_table(SessionDB, ets:first(SessionDB), Transform), @@ -678,35 +670,42 @@ get_manager_info(#state{handler_db = HDB, CookieInfo = httpc_cookie:which_cookies(CDB), [{handlers, HandlerInfo}, {cookies, CookieInfo}]. +sort_handlers(Unsorted) -> + sort_handlers2(lists:keysort(1, Unsorted)). + +sort_handlers2([]) -> + []; +sort_handlers2([{HandlerPid, RequestId}|L]) -> + {Handler, Rest} = sort_handlers2(HandlerPid, [RequestId], L), + [Handler | sort_handlers2(Rest)]. + +sort_handlers2(HandlerPid, Reqs, []) -> + {{HandlerPid, lists:sort(Reqs)}, []}; +sort_handlers2(HandlerPid, Reqs, [{HandlerPid, ReqId}|Rest]) -> + sort_handlers2(HandlerPid, [ReqId|Reqs], Rest); +sort_handlers2(HandlerPid1, Reqs, [{HandlerPid2, _}|_] = Rest) + when HandlerPid1 =/= HandlerPid2 -> + {{HandlerPid1, lists:sort(Reqs)}, Rest}. + get_handler_info(Tab) -> - Pattern = #handler_info{handler = '$1', - state = '$2', - _ = '_'}, - Handlers1 = [{Pid, State} || [Pid, State] <- ets:match(Tab, Pattern)], - F = fun({Pid, State} = Elem, Acc) when State =/= canceled -> - case lists:keymember(Pid, 1, Acc) of - true -> - Acc; - false -> - [Elem | Acc] - end; - (_, Acc) -> - Acc - end, - Handlers2 = lists:foldl(F, [], Handlers1), - Handlers3 = [{Pid, State, - case (catch httpc_handler:info(Pid)) of - {'EXIT', _} -> + Pattern = {'$2', '$1', '_'}, + Handlers1 = [{Pid, Id} || [Pid, Id] <- ets:match(Tab, Pattern)], + Handlers2 = sort_handlers(Handlers1), + Handlers3 = [{Pid, Reqs, + try + begin + httpc_handler:info(Pid) + end + catch + _:_ -> %% Why would this crash? %% Only if the process has died, but we don't %% know about it? - []; - Else -> - Else - end} || - {Pid, State} <- Handlers2], + [] + end} || {Pid, Reqs} <- Handlers2], Handlers3. + handle_request(#request{settings = #http_options{version = "HTTP/0.9"}} = Request, State) -> @@ -758,19 +757,21 @@ handle_request(Request, State = #state{options = Options}) -> {reply, {ok, NewRequest#request.id}, State}. -start_handler(Request, State) -> +start_handler(#request{id = Id, + from = From} = Request, + #state{profile_name = ProfileName, + handler_db = HandlerDb, + options = Options}) -> {ok, Pid} = case is_inets_manager() of true -> httpc_handler_sup:start_child([whereis(httpc_handler_sup), - Request, State#state.options, - State#state.profile_name]); + Request, Options, ProfileName]); false -> - httpc_handler:start_link(self(), Request, State#state.options, - State#state.profile_name) + httpc_handler:start_link(self(), Request, Options, ProfileName) end, - ets:insert(State#state.handler_db, {Request#request.id, - Pid, Request#request.from}), + HandlerInfo = {Id, Pid, From}, + ets:insert(HandlerDb, HandlerInfo), erlang:monitor(process, Pid). @@ -827,12 +828,14 @@ select_session(Candidates, Max) -> {ok, HandlerPid} end. -pipeline_or_keep_alive(Request, HandlerPid, State) -> +pipeline_or_keep_alive(#request{id = Id, + from = From} = Request, + HandlerPid, + #state{handler_db = HandlerDb} = State) -> case (catch httpc_handler:send(Request, HandlerPid)) of ok -> - ets:insert(State#state.handler_db, {Request#request.id, - HandlerPid, - Request#request.from}); + HandlerInfo = {Id, HandlerPid, From}, + ets:insert(HandlerDb, HandlerInfo); _ -> % timeout pipelining failed start_handler(Request, State) end. diff --git a/lib/inets/test/httpc_SUITE.erl b/lib/inets/test/httpc_SUITE.erl index c4943cbb4c..1cdd96f0b0 100644 --- a/lib/inets/test/httpc_SUITE.erl +++ b/lib/inets/test/httpc_SUITE.erl @@ -768,120 +768,83 @@ test_pipeline(URL) -> p("test_pipeline -> issue (async) request 1" "~n when profile info: ~p", [httpc:info()]), - {ok, RequestId1} = + {ok, RequestIdA1} = httpc:request(get, {URL, []}, [], [{sync, false}]), - tsp("RequestId1: ~p", [RequestId1]), - p("test_pipeline -> RequestId1: ~p" - "~n when profile info: ~p", [RequestId1, httpc:info()]), + tsp("RequestIdA1: ~p", [RequestIdA1]), + p("test_pipeline -> RequestIdA1: ~p" + "~n when profile info: ~p", [RequestIdA1, httpc:info()]), %% Make sure pipeline is initiated p("test_pipeline -> sleep some", []), test_server:sleep(4000), - p("test_pipeline -> issue (async) request 2" + p("test_pipeline -> issue (async) request A2, A3 and A4" "~n when profile info: ~p", [httpc:info()]), - {ok, RequestId2} = + {ok, RequestIdA2} = httpc:request(get, {URL, []}, [], [{sync, false}]), - tsp("RequestId2: ~p", [RequestId2]), - p("test_pipeline -> RequestId2: ~p" - "~n when profile info: ~p", [RequestId2, httpc:info()]), + {ok, RequestIdA3} = + httpc:request(get, {URL, []}, [], [{sync, false}]), + {ok, RequestIdA4} = + httpc:request(get, {URL, []}, [], [{sync, false}]), + tsp("RequestIdAs => A2: ~p, A3: ~p and A4: ~p", + [RequestIdA2, RequestIdA3, RequestIdA4]), + p("test_pipeline -> RequestIds => A2: ~p, A3: ~p and A4: ~p" + "~n when profile info: ~p", + [RequestIdA2, RequestIdA3, RequestIdA4, httpc:info()]), p("test_pipeline -> issue (sync) request 3"), {ok, {{_,200,_}, [_ | _], [_ | _]}} = httpc:request(get, {URL, []}, [], []), - p("test_pipeline -> expect reply for (async) request 1 or 2" + p("test_pipeline -> expect reply for (async) request A1, A2, A3 and A4" "~n when profile info: ~p", [httpc:info()]), - receive - {http, {RequestId1, {{_, 200, _}, _, _}}} -> - p("test_pipeline -> " - "received reply for (async) request 1 - now wait for 2" - "~n when profile info: ~p", [httpc:info()]), - receive - {http, {RequestId2, {{_, 200, _}, _, _}}} -> - p("test_pipeline -> " - "received reply for (async) request 2" - "~n when profile info: ~p", [httpc:info()]), - ok; - {http, Msg1} -> - tsf(Msg1) - end; - {http, {RequestId2, {{_, 200, _}, _, _}}} -> - io:format("test_pipeline -> " - "received reply for (async) request 2 - now wait for 1" - "~n when profile info: ~p", [httpc:info()]), - receive - {http, {RequestId1, {{_, 200, _}, _, _}}} -> - io:format("test_pipeline -> " - "received reply for (async) request 1" - "~n when profile info: ~p", [httpc:info()]), - ok; - {http, Msg2} -> - tsf(Msg2) - end; - {http, Msg3} -> - tsf(Msg3) - after 60000 -> - receive Any1 -> - tsp("received crap after timeout: ~n ~p", [Any1]), - tsf({error, {timeout, Any1}}) - end - end, - + pipeline_await_async_reply([{RequestIdA1, a1, 200}, + {RequestIdA2, a2, 200}, + {RequestIdA3, a3, 200}, + {RequestIdA4, a4, 200}], ?MINS(1)), + p("test_pipeline -> sleep some" "~n when profile info: ~p", [httpc:info()]), test_server:sleep(4000), - p("test_pipeline -> issue (async) request 4" + p("test_pipeline -> issue (async) request B1, B2, B3 and B4" "~n when profile info: ~p", [httpc:info()]), - {ok, RequestId3} = + {ok, RequestIdB1} = httpc:request(get, {URL, []}, [], [{sync, false}]), - tsp("RequestId3: ~p", [RequestId3]), - p("test_pipeline -> RequestId3: ~p" - "~n when profile info: ~p", [RequestId3, httpc:info()]), - - p("test_pipeline -> issue (async) request 5" - "~n when profile info: ~p", [httpc:info()]), - {ok, RequestId4} = + {ok, RequestIdB2} = + httpc:request(get, {URL, []}, [], [{sync, false}]), + {ok, RequestIdB3} = httpc:request(get, {URL, []}, [], [{sync, false}]), - tsp("RequestId4: ~p", [RequestId4]), - p("test_pipeline -> RequestId4: ~p" - "~n when profile info: ~p", [RequestId4, httpc:info()]), + {ok, RequestIdB4} = + httpc:request(get, {URL, []}, [], [{sync, false}]), + tsp("RequestIdBs => B1: ~p, B2: ~p, B3: ~p and B4: ~p", + [RequestIdB1, RequestIdB2, RequestIdB3, RequestIdB4]), + p("test_pipeline -> RequestIdBs => B1: ~p, B2: ~p, B3: ~p and B4: ~p" + "~n when profile info: ~p", + [RequestIdB1, RequestIdB2, RequestIdB3, RequestIdB4, httpc:info()]), - p("test_pipeline -> cancel (async) request 4" + p("test_pipeline -> cancel (async) request B2" "~n when profile info: ~p", [httpc:info()]), - ok = httpc:cancel_request(RequestId3), + ok = httpc:cancel_request(RequestIdB2), p("test_pipeline -> " - "expect *no* reply for cancelled (async) request 4 (for 3 secs)" + "expect *no* reply for cancelled (async) request B2 (for 3 secs)" "~n when profile info: ~p", [httpc:info()]), receive - {http, {RequestId3, _}} -> + {http, {RequestIdB2, _}} -> tsf(http_cancel_request_failed) after 3000 -> ok end, - p("test_pipeline -> expect reply for (async) request 4" + p("test_pipeline -> expect reply for (async) request B1, B3 and B4" "~n when profile info: ~p", [httpc:info()]), - Body = - receive - {http, {RequestId4, {{_, 200, _}, _, BinBody4}}} = Res -> - p("test_pipeline -> " - "received reply for (async) request 5" - "~n when profile info: ~p", [httpc:info()]), - tsp("Receive : ~p", [Res]), - BinBody4; - {http, Msg4} -> - tsf(Msg4) - after 60000 -> - receive Any2 -> - tsp("received crap after timeout: ~n ~p", [Any2]), - tsf({error, {timeout, Any2}}) - end - end, - - p("test_pipeline -> check reply for (async) request 5" + Bodies = pipeline_await_async_reply([{RequestIdB1, b1, 200}, + {RequestIdB3, b3, 200}, + {RequestIdB4, b4, 200}], ?MINS(1)), + [{b1, Body}|_] = Bodies, + + p("test_pipeline -> check reply for (async) request B1" "~n when profile info: ~p", [httpc:info()]), inets_test_lib:check_body(binary_to_list(Body)), @@ -898,6 +861,58 @@ test_pipeline(URL) -> "~n when profile info: ~p", [httpc:info()]), ok. +pipeline_await_async_reply(ReqIds, Timeout) -> + pipeline_await_async_reply(ReqIds, Timeout, []). + +pipeline_await_async_reply([], _, Acc) -> + lists:keysort(1, Acc); +pipeline_await_async_reply(ReqIds, Timeout, Acc) when Timeout > 0 -> + T1 = inets_test_lib:timestamp(), + p("pipeline_await_async_reply -> await replies" + "~n ReqIds: ~p" + "~n Timeout: ~p", [ReqIds, Timeout]), + receive + {http, {RequestId, {{_, Status, _}, _, Body}}} -> + p("pipeline_await_async_reply -> received reply for" + "~n RequestId: ~p" + "~n Status: ~p", [RequestId, Status]), + case lists:keysearch(RequestId, 1, ReqIds) of + {value, {RequestId, N, Status}} -> + p("pipeline_await_async_reply -> " + "found expected request ~w", [N]), + ReqIds2 = lists:keydelete(RequestId, 1, ReqIds), + NewTimeout = Timeout - (inets_test_lib:timestamp()-T1), + pipeline_await_async_reply(ReqIds2, NewTimeout, + [{N, Body} | Acc]); + {value, {RequestId, N, WrongStatus}} -> + p("pipeline_await_async_reply -> " + "found request ~w with wrong status", [N]), + tsf({reply_with_unexpected_status, + {RequestId, N, WrongStatus}}); + false -> + tsf({unexpected_reply, {RequestId, Status}}) + end; + {http, Msg} -> + tsf({unexpected_reply, Msg}) + after Timeout -> + receive + Any -> + tsp("pipeline_await_async_reply -> " + "received unknown data after timeout: " + "~n ~p", [Any]), + tsf({timeout, {unknown, Any}}) + end + end; +pipeline_await_async_reply(ReqIds, _, Acc) -> + tsp("pipeline_await_async_reply -> " + "timeout: " + "~n ~p" + "~nwhen" + "~n ~p", [ReqIds, Acc]), + tsf({timeout, ReqIds, Acc}). + + + %%------------------------------------------------------------------------- http_trace(doc) -> ["Perform a TRACE request that goes through a proxy."]; diff --git a/lib/inets/test/inets_test_lib.erl b/lib/inets/test/inets_test_lib.erl index c94be796cd..7f4b0ec8d8 100644 --- a/lib/inets/test/inets_test_lib.erl +++ b/lib/inets/test/inets_test_lib.erl @@ -31,6 +31,7 @@ send/3, close/2]). -export([copy_file/3, copy_files/2, copy_dirs/2, del_dirs/1]). -export([info/4, log/4, debug/4, print/4]). +-export([timestamp/0, formated_timestamp/0]). -export([tsp/1, tsp/2, tsf/1, tss/1]). -export([check_body/1]). -export([millis/0, millis_diff/2, hours/1, minutes/1, seconds/1, sleep/1]). @@ -642,6 +643,9 @@ tsf(Reason) -> tss(Time) -> test_server:sleep(Time). +timestamp() -> + http_util:timestamp(). + formated_timestamp() -> format_timestamp( os:timestamp() ). -- cgit v1.2.3