diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index a4def5b27077e..565d5c58b4eee 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,9 @@ +* Make ActiveRecord `ConnectionPool.connections` method thread-safe. + + Fixes #36465. + + *Jeff Doering* + * Add support for multiple databases to `rails db:abort_if_pending_migrations`. *Mark Lee* 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 be804325b33d8..adb5ae82a0f6e 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -357,7 +357,7 @@ def run include ConnectionAdapters::AbstractPool attr_accessor :automatic_reconnect, :checkout_timeout, :schema_cache - attr_reader :spec, :connections, :size, :reaper + attr_reader :spec, :size, :reaper # Creates a new ConnectionPool object. +spec+ is a ConnectionSpecification # object which describes database connection information (e.g. adapter, @@ -470,6 +470,21 @@ def connected? synchronize { @connections.any? } end + # Returns an array containing the connections currently in the pool. + # Access to the array does not require synchronization on the pool because + # the array is newly created and not retained by the pool. + # + # However; this method bypasses the ConnectionPool's thread-safe connection + # access pattern. A returned connection may be owned by another thread, + # unowned, or by happen-stance owned by the calling thread. + # + # Calling methods on a connection without ownership is subject to the + # thread-safety guarantees of the underlying method. Many of the methods + # on connection adapter classes are inherently multi-thread unsafe. + def connections + synchronize { @connections.dup } + end + # Disconnects all connections in the pool, and clears the pool. # # Raises: diff --git a/activerecord/test/cases/connection_pool_test.rb b/activerecord/test/cases/connection_pool_test.rb index 82f5f45a4c0ae..ccbb6e16cdcc6 100644 --- a/activerecord/test/cases/connection_pool_test.rb +++ b/activerecord/test/cases/connection_pool_test.rb @@ -694,6 +694,28 @@ def test_connection_pool_stat end end + def test_public_connections_access_threadsafe + _conn1 = @pool.checkout + conn2 = @pool.checkout + + connections = @pool.connections + found_conn = nil + + # Without assuming too much about implementation + # details make sure that a concurrent change to + # the pool is thread-safe. + connections.each_index do |idx| + if connections[idx] == conn2 + Thread.new do + @pool.remove(conn2) + end.join + end + found_conn = connections[idx] + end + + assert_not_nil found_conn + end + private def with_single_connection_pool one_conn_spec = ActiveRecord::Base.connection_pool.spec.dup