Make ActiveRecord `ConnectionPool.connections` thread-safe. (#36473)

* Make ActiveRecord `ConnectionPool.connections` thread-safe.

ConnectionPool documentation is clear on the need to synchronize
access to @connections but also states that public methods do not
require synchronization. Existing code exposed @connections
directly via attr_reader. The fix uses synchronize() to lock
@connections then returns a copy to the caller using Array.dup().

Includes comments on the connections method that thread-safe access
to the connections array does not imply thread-safety of accessing
methods on the actual connections.

Adds a test-case that modifies the pool using a supported method
in one thread  while a second thread accesses pool.connections.
The test fails without this patch.

Fixes #36465.

* Update activerecord/test/cases/connection_pool_test.rb

[jeffdoering + Rafael Mendonça França]

1 files changed, 22 insertions, 0 deletions

diff --git a/activerecord/test/cases/connection_pool_test.rb b/activerecord/test/cases/connection_pool_test.rb
index 82f5f45a4c..ccbb6e16cd 100644
--- a/activerecord/test/cases/connection_pool_test.rb
+++ b/activerecord/test/cases/connection_pool_test.rb
@@ -694,6 +694,28 @@ module ActiveRecord
         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