From 313362c56b9617e925ae7e77186f98a631b3680a Mon Sep 17 00:00:00 2001
From: Jay Doane <jaydoane@apache.org>
Date: Mon, 8 Apr 2019 23:43:12 -0700
Subject: [PATCH] Get a new lb pid if existing pid is not alive

It's possible for the connection process associated with a pid in the
ibrowse_lb ets table to die, yet remain in the table, in which case
subsequent requests to the corresponding {Host, Port} will result in an
error like the following:

(node1@127.0.0.1)9> ibrowse:send_req("http://localhost:15984", [], get).
                ** exception exit: {noproc,
                       {gen_server,call,
                           [<0.2451.0>,
                            {spawn_connection,
                                {url,"http://localhost:15984","localhost",15984,
                                    undefined,undefined,"/",http,hostname},
                                10,10,
                                {[],false},
                                []}]}}
     in function  gen_server:call/2 (gen_server.erl, line 215)
     in call from ibrowse:try_routing_request/14 (src/ibrowse.erl, line 377)

This checks whether the pid about to be returned from the table is alive,
and if not, the entry is deleted, and a new pid is obtained.
---
 src/ibrowse.erl       | 21 +++++++++++++++------
 test/ibrowse_test.erl | 19 +++++++++++++++++++
 2 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/src/ibrowse.erl b/src/ibrowse.erl
index 2066799..5adf047 100644
--- a/src/ibrowse.erl
+++ b/src/ibrowse.erl
@@ -342,12 +342,7 @@ send_req(Url, Headers, Method, Body, Options, Timeout) ->
         #url{host = Host,
              port = Port,
              protocol = Protocol} = Parsed_url ->
-            Lb_pid = case ets:lookup(ibrowse_lb, {Host, Port}) of
-                         [] ->
-                             get_lb_pid(Parsed_url);
-                         [#lb_pid{pid = Lb_pid_1}] ->
-                             Lb_pid_1
-                     end,
+            Lb_pid = lb_pid(Host, Port, Parsed_url),
             Max_sessions = get_max_sessions(Host, Port, Options),
             Max_pipeline_size = get_max_pipeline_size(Host, Port, Options),
             Max_attempts = get_max_attempts(Host, Port, Options),
@@ -367,6 +362,20 @@ send_req(Url, Headers, Method, Body, Options, Timeout) ->
             {error, {url_parsing_failed, Err}}
     end.
 
+lb_pid(Host, Port, Url) ->
+    case ets:lookup(ibrowse_lb, {Host, Port}) of
+        [] ->
+            get_lb_pid(Url);
+        [#lb_pid{pid = Pid}] ->
+            case is_process_alive(Pid) of
+                true ->
+                    Pid;
+                false ->
+                    ets:delete(ibrowse_lb, {Host, Port}),
+                    get_lb_pid(Url)
+            end
+    end.
+
 try_routing_request(Lb_pid, Parsed_url,
                     Max_sessions, 
                     Max_pipeline_size,
diff --git a/test/ibrowse_test.erl b/test/ibrowse_test.erl
index cd78049..4e64acc 100644
--- a/test/ibrowse_test.erl
+++ b/test/ibrowse_test.erl
@@ -34,6 +34,7 @@
          test_preserve_status_line/0,
          test_binary_headers/0,
          test_binary_headers/1,
+         test_dead_lb_pid/0,
          test_generate_body_0/0,
          test_retry_of_requests/0,
          test_retry_of_requests/1,
@@ -61,6 +62,7 @@
                       {local_test_fun, test_303_response_with_a_body, []},
 		      {local_test_fun, test_303_response_with_no_body, []},
                       {local_test_fun, test_binary_headers, []},
+                      {local_test_fun, test_dead_lb_pid, []},
                       {local_test_fun, test_retry_of_requests, []},
 		      {local_test_fun, verify_chunked_streaming, []},
 		      {local_test_fun, test_chunked_streaming_once, []},
@@ -877,6 +879,23 @@ test_generate_body_0() ->
         ets:delete(Tid)
     end.
 
+%%------------------------------------------------------------------------------
+%% Test that when an lb process dies, its entry is removed from the ibrowse_lb
+%% table by the next requestor and replaced with a new process
+%%------------------------------------------------------------------------------
+test_dead_lb_pid() ->
+    {Host, Port} = {"localhost", 8181},
+    Url = "http://" ++ Host ++ ":" ++ integer_to_list(Port),
+    {ok, "200", _, _} = ibrowse:send_req(Url, [], get),
+    [{lb_pid, {Host, Port}, Pid, _}] = ets:lookup(ibrowse_lb, {Host, Port}),
+    true = exit(Pid, kill),
+    false = is_process_alive(Pid),
+    {ok, "200", _, _} = ibrowse:send_req(Url, [], get),
+    [{lb_pid, {Host, Port}, NewPid, _}] = ets:lookup(ibrowse_lb, {Host, Port}),
+    true = NewPid /= Pid,
+    true = is_process_alive(NewPid),
+    success.
+
 do_trace(Fmt, Args) ->
     do_trace(get(my_trace_flag), Fmt, Args).