Split connection handler into single- and multiple-thread versions.

2 files changed, 57 insertions, 31 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 01a75365d3..ca9f0a5d9b 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -134,18 +134,17 @@ module ActiveRecord
         end
     end
 
-    class ConnectionHandler
-      attr_reader :connection_pools_lock
+    module ConnectionHandlerMethods
+      def initialize(pools = {})
+        @connection_pools = pools
+      end
 
-      def initialize
-        @connection_pools = {}
-        @connection_pools_lock = Monitor.new
+      def connection_pools
+        @connection_pools ||= {}
       end
 
       def establish_connection(name, spec)
-        connection_pools_lock.synchronize do
-          @connection_pools[name] = ConnectionAdapters::ConnectionPool.new(spec)
-        end
+        @connection_pools[name] = ConnectionAdapters::ConnectionPool.new(spec)
       end
 
       # for internal use only and for testing
@@ -168,7 +167,7 @@ module ActiveRecord
       end
 
       def clear_all_connections!
-        clear_cache!(@connection_pools) {|name, pool| pool.disconnect! }
+        @connection_pools.each_value {|pool| pool.disconnect! }
       end
 
       # Verify active connections.
@@ -185,15 +184,6 @@ module ActiveRecord
         (pool && pool.connection) or raise ConnectionNotEstablished
       end
 
-      def retrieve_connection_pool(klass)
-        loop do
-          pool = @connection_pools[klass.name]
-          return pool if pool
-          return nil if ActiveRecord::Base == klass
-          klass = klass.superclass
-        end
-      end
-
       # Returns true if a connection that's accessible to this class has already been opened.
       def connected?(klass)
         retrieve_connection_pool(klass).connected?
@@ -210,16 +200,40 @@ module ActiveRecord
         pool.spec.config if pool
       end
 
-      synchronize :retrieve_connection, :retrieve_connection_pool, :connected?,
-        :remove_connection, :active_connections, :clear_active_connections!,
-        :clear_reloadable_connections!, :clear_all_connections!,
-        :verify_active_connections!, :with => :connection_pools_lock
-
       private
-        def clear_cache!(cache, &block)
-          cache.each(&block) if block_given?
-          cache.clear
+        def retrieve_connection_pool(klass)
+          loop do
+            pool = @connection_pools[klass.name]
+            return pool if pool
+            return nil if ActiveRecord::Base == klass
+            klass = klass.superclass
+          end
         end
     end
+
+    # This connection handler is not thread-safe, as it does not protect access
+    # to the underlying connection pools.
+    class SingleThreadConnectionHandler
+      include ConnectionHandlerMethods
+    end
+
+    # This connection handler is thread-safe. Each access or modification of a thread
+    # pool is synchronized by an internal monitor.
+    class MultipleThreadConnectionHandler
+      attr_reader :connection_pools_lock
+      include ConnectionHandlerMethods
+
+      def initialize(pools = {})
+        super
+        @connection_pools_lock = Monitor.new
+      end
+
+      # Apply monitor to all public methods that access the pool.
+      synchronize :establish_connection, :retrieve_connection,
+        :connected?, :remove_connection, :active_connections,
+        :clear_active_connections!, :clear_reloadable_connections!,
+        :clear_all_connections!, :verify_active_connections!,
+        :with => :connection_pools_lock
+    end
   end
 end
\ No newline at end of file
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
index 2dc3201208..0ebb191381 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
@@ -14,12 +14,24 @@ module ActiveRecord
 
     # The connection handler
     cattr_accessor :connection_handler, :instance_writer => false
-    @@connection_handler = ConnectionAdapters::ConnectionHandler.new
+    @@connection_handler = ConnectionAdapters::SingleThreadConnectionHandler.new
 
-    # Turning on allow_concurrency basically switches a null mutex for a real one, so that
-    # multi-threaded access of the connection pools hash is synchronized.
+    # Turning on allow_concurrency changes the single threaded connection handler
+    # for a multiple threaded one, so that multi-threaded access of the
+    # connection pools is synchronized.
     def self.allow_concurrency=(flag)
-      @@allow_concurrency = flag
+      if @@allow_concurrency != flag
+        @@allow_concurrency = flag
+        # When switching connection handlers, preserve the existing pools so that
+        # #establish_connection doesn't need to be called again.
+        if @@allow_concurrency
+          self.connection_handler = ConnectionAdapters::MultipleThreadConnectionHandler.new(
+            self.connection_handler.connection_pools)
+        else
+          self.connection_handler = ConnectionAdapters::SingleThreadConnectionHandler.new(
+            self.connection_handler.connection_pools)
+        end
+      end
     end
 
     # Returns the connection currently associated with the class. This can
@@ -112,7 +124,7 @@ module ActiveRecord
         connection_handler.connected?(self)
       end
 
-      def remove_connection(klass=self)
+      def remove_connection(klass = self)
         connection_handler.remove_connection(klass)
       end