Fixed bug with connection req completion

Algorithm change had bug where ets:select return
value was incorrectly assumed to be the object key
and not the entire object causing the following
delete attempt based on a matchspec and the key to fail.

This meant that ets was not updated to reflect the completed
requests on each connection and causing exaustion of pipelines
event though connections were idle.  Functional test added which
demonstrated the problem.

src/ibrowse_lb.erl

@@ -259,7 +259,7 @@ report_request_complete(_Tid, 0) ->
 report_request_complete(Tid, RemainingRetries) ->
     %% Don't cascade errors since Tid is really managed by other process
     catch case ets:select(Tid, ?KEY_MATCHSPEC_BY_PID(self())) of
-        [MatchKey] ->
+        [{MatchKey, _}] ->
             case ets:select_delete(Tid, ?KEY_MATCHSPEC_FOR_DELETE(MatchKey)) of
                 1 ->
                     ets:insert(Tid, {decremented(MatchKey), undefined}),

test/ibrowse_functional_tests.erl

@@ -38,6 +38,7 @@ running_server_fixture_test_() ->
         ?TIMEDTEST("Simple request can be honored", simple_request),
         ?TIMEDTEST("Slow server causes timeout", slow_server_timeout),
         ?TIMEDTEST("Pipeline depth goes down with responses", pipeline_depth),
+        ?TIMEDTEST("Pipelines refill", pipeline_refill),
         ?TIMEDTEST("Timeout closes pipe", closing_pipes),
         ?TIMEDTEST("Requests are balanced over connections", balanced_connections),
         ?TIMEDTEST("Pipeline too small signals retries", small_pipeline),
@@ -68,6 +69,26 @@ pipeline_depth() ->
     ?assertEqual(MaxSessions, length(Counts)),
     ?assertEqual(lists:duplicate(MaxSessions, EmptyPipelineDepth), Counts).
 
+pipeline_refill() ->
+    MaxSessions = 2,
+    MaxPipeline = 2,
+    RequestsToFill = MaxSessions * MaxPipeline,
+
+    %% Send off enough requests to fill sessions and pipelines in rappid succession
+    Fun = fun() -> ibrowse:send_req(?BASE_URL, [], get, [], [{max_sessions, MaxSessions}, {max_pipeline_size, MaxPipeline}], ?SHORT_TIMEOUT_MS) end,
+    times(RequestsToFill, fun() -> spawn_link(Fun) end),
+    timer:sleep(?PAUSE_FOR_CONNECTIONS_MS),
+
+    % Verify that connections properly reported their completed responses and can still accept more
+    ?assertMatch({ok, "200", _, _}, ibrowse:send_req(?BASE_URL, [], get, [], [{max_sessions, MaxSessions}, {max_pipeline_size, MaxPipeline}], ?SHORT_TIMEOUT_MS)),
+
+    % and do it again to make sure we really are clear
+    times(RequestsToFill, fun() -> spawn_link(Fun) end),
+    timer:sleep(?PAUSE_FOR_CONNECTIONS_MS),
+
+    % Verify that connections properly reported their completed responses and can still accept more
+    ?assertMatch({ok, "200", _, _}, ibrowse:send_req(?BASE_URL, [], get, [], [{max_sessions, MaxSessions}, {max_pipeline_size, MaxPipeline}], ?SHORT_TIMEOUT_MS)).
+
 closing_pipes() ->
     MaxSessions = 2,
     MaxPipeline = 2,