1 files changed, 21 insertions, 16 deletions
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
index 9f660a419d..6c06f67239 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require "thread"
 require "concurrent/map"
 require "monitor"
@@ -265,7 +267,7 @@ module ActiveRecord
       # Connections must be leased while holding the main pool mutex. This is
       # an internal subclass that also +.leases+ returned connections while
       # still in queue's critical section (queue synchronizes with the same
-      # +@lock+ as the main pool) so that a returned connection is already
+      # <tt>@lock</tt> as the main pool) so that a returned connection is already
       # leased and there is no need to re-enter synchronized block.
       class ConnectionLeasingQueue < Queue # :nodoc:
         include BiasableQueue
@@ -321,8 +323,6 @@ module ActiveRecord
         @spec = spec
 
         @checkout_timeout = (spec.config[:checkout_timeout] && spec.config[:checkout_timeout].to_f) || 5
-        @reaper = Reaper.new(self, (spec.config[:reaping_frequency] && spec.config[:reaping_frequency].to_f))
-        @reaper.run
 
         # default max pool size to 5
         @size = (spec.config[:pool] && spec.config[:pool].to_i) || 5
@@ -335,7 +335,7 @@ module ActiveRecord
         # then that +thread+ does indeed own that +conn+. However, an absence of a such
         # mapping does not mean that the +thread+ doesn't own the said connection. In
         # that case +conn.owner+ attr should be consulted.
-        # Access and modification of +@thread_cached_conns+ does not require
+        # Access and modification of <tt>@thread_cached_conns</tt> does not require
         # synchronization.
         @thread_cached_conns = Concurrent::Map.new(initial_capacity: @size)
 
@@ -352,6 +352,9 @@ module ActiveRecord
         @available = ConnectionLeasingQueue.new self
 
         @lock_thread = false
+
+        @reaper = Reaper.new(self, spec.config[:reaping_frequency] && spec.config[:reaping_frequency].to_f)
+        @reaper.run
       end
 
       def lock_thread=(lock_thread)
@@ -503,14 +506,16 @@ module ActiveRecord
       # +conn+: an AbstractAdapter object, which was obtained by earlier by
       # calling #checkout on this pool.
       def checkin(conn)
-        synchronize do
-          remove_connection_from_thread_cache conn
+        conn.lock.synchronize do
+          synchronize do
+            remove_connection_from_thread_cache conn
 
-          conn._run_checkin_callbacks do
-            conn.expire
-          end
+            conn._run_checkin_callbacks do
+              conn.expire
+            end
 
-          @available.add conn
+            @available.add conn
+          end
         end
       end
 
@@ -674,7 +679,7 @@ module ActiveRecord
           # this block can't be easily moved into attempt_to_checkout_all_existing_connections's
           # rescue block, because doing so would put it outside of synchronize section, without
           # being in a critical section thread_report might become inaccurate
-          msg = "could not obtain ownership of all database connections in #{checkout_timeout} seconds"
+          msg = "could not obtain ownership of all database connections in #{checkout_timeout} seconds".dup
 
           thread_report = []
           @connections.each do |conn|
@@ -729,10 +734,10 @@ module ActiveRecord
         # Implementation detail: the connection returned by +acquire_connection+
         # will already be "+connection.lease+ -ed" to the current thread.
         def acquire_connection(checkout_timeout)
-          # NOTE: we rely on +@available.poll+ and +try_to_checkout_new_connection+ to
+          # NOTE: we rely on <tt>@available.poll</tt> and +try_to_checkout_new_connection+ to
           # +conn.lease+ the returned connection (and to do this in a +synchronized+
           # section). This is not the cleanest implementation, as ideally we would
-          # <tt>synchronize { conn.lease }</tt> in this method, but by leaving it to +@available.poll+
+          # <tt>synchronize { conn.lease }</tt> in this method, but by leaving it to <tt>@available.poll</tt>
           # and +try_to_checkout_new_connection+ we can piggyback on +synchronize+ sections
           # of the said methods and avoid an additional +synchronize+ overhead.
           if conn = @available.poll || try_to_checkout_new_connection
@@ -756,7 +761,7 @@ module ActiveRecord
           end
         end
 
-        # If the pool is not at a +@size+ limit, establish new connection. Connecting
+        # If the pool is not at a <tt>@size</tt> limit, establish new connection. Connecting
         # to the DB is done outside main synchronized section.
         #--
         # Implementation constraint: a newly established connection returned by this
@@ -854,9 +859,9 @@ module ActiveRecord
     # All Active Record models use this handler to determine the connection pool that they
     # should use.
     #
-    # The ConnectionHandler class is not coupled with the Active models, as it has no knowlodge
+    # The ConnectionHandler class is not coupled with the Active models, as it has no knowledge
     # about the model. The model needs to pass a specification name to the handler,
-    # in order to lookup the correct connection pool.
+    # in order to look up the correct connection pool.
     class ConnectionHandler
       def initialize
         # These caches are keyed by spec.name (ConnectionSpecification#name).