Retry new connection on checkout after reap

When we reap connections, we check if they are inactive (connected and
responding to ping in most adapters) and if so we remove the connection
instead of checking it back in.

However, in acquire_connection, we weren't checking after reaping
whether we were allowed to build a new connection, only whether an
existing one was in the available pool.

This still leaves a race condition where if the background reaper thread
runs while a thread is polling and finds a free inactive connection, it
could have the same issue and not wake the waiting thread. However we
don't really expect the reaper to solve this (it only runs every 60
seconds by default, far too slow to solve for a blocked thread). I think
this should be fixed, just separately.

Author: jhawthorn

activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb

@@ -668,7 +668,13 @@ def acquire_connection(checkout_timeout)
             conn
           else
             reap
-            @available.poll(checkout_timeout)
+            # Retry after reaping, which may return an available connection,
+            # remove an inactive connection, or both
+            if conn = @available.poll || try_to_checkout_new_connection
+              conn
+            else
+              @available.poll(checkout_timeout)
+            end
           end
         end
 

activerecord/test/cases/connection_pool_test.rb

@@ -215,6 +215,30 @@ def test_reap_inactive
         @pool.connections.each { |conn| conn.close if conn.in_use? }
       end
 
+      def test_inactive_are_returned_from_dead_thread
+        ready = Concurrent::CountDownLatch.new
+        @pool.instance_variable_set(:@size, 1)
+
+        child = new_thread do
+          @pool.checkout
+          ready.count_down
+          stop_thread
+        end
+
+        pass_to(child) until ready.wait(0)
+
+        assert_equal 1, active_connections(@pool).size
+
+        child.terminate
+        child.join
+
+        @pool.checkout
+
+        assert_equal 1, active_connections(@pool).size
+      ensure
+        @pool.connections.each { |conn| conn.close if conn.in_use? }
+      end
+
       def test_idle_timeout_configuration
         @pool.disconnect!