Skip to content
This repository
Browse code

Ensure ActiveRecord::Base.connection_pool.with_connection creates a n…

…ew connection only when needed [#1752 state:resolved]

Signed-off-by: Pratik Naik <pratiknaik@gmail.com>
  • Loading branch information...
commit 5501b99a19a2a67a9a920fd3c7bff071a2ecf058 1 parent b193f23
coderrr authored May 01, 2009 lifo committed May 01, 2009
11  activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -107,13 +107,14 @@ def release_connection
107 107
         checkin conn if conn
108 108
       end
109 109
 
110  
-      # Reserve a connection, and yield it to a block. Ensure the connection is
111  
-      # checked back in when finished.
  110
+      # If a connection already exists yield it to the block.  If no connection
  111
+      # exists checkout a connection, yield it to the block, and checkin the 
  112
+      # connection when finished.
112 113
       def with_connection
113  
-        conn = checkout
114  
-        yield conn
  114
+        fresh_connection = true unless @reserved_connections[current_connection_id]
  115
+        yield connection
115 116
       ensure
116  
-        checkin conn
  117
+        release_connection if fresh_connection
117 118
       end
118 119
 
119 120
       # Returns true if a connection has already been opened.
29  activerecord/test/cases/pooled_connections_test.rb
... ...
@@ -1,4 +1,6 @@
1 1
 require "cases/helper"
  2
+require "models/project"
  3
+require "timeout"
2 4
 
3 5
 class PooledConnectionsTest < ActiveRecord::TestCase
4 6
   def setup
@@ -89,6 +91,33 @@ def test_undefined_connection_returns_false
89 91
   ensure
90 92
     ActiveRecord::Base.connection_handler = old_handler
91 93
   end
  94
+
  95
+  def test_with_connection_nesting_safety
  96
+    ActiveRecord::Base.establish_connection(@connection.merge({:pool => 1, :wait_timeout => 0.1}))
  97
+
  98
+    before_count = Project.count
  99
+
  100
+    add_record('one')
  101
+
  102
+    ActiveRecord::Base.connection.transaction do
  103
+      add_record('two')
  104
+      # Have another thread try to screw up the transaction
  105
+      Thread.new do
  106
+        raise ActiveRecord::Rollback
  107
+        ActiveRecord::Base.connection_pool.release_connection
  108
+      end.join rescue nil
  109
+      add_record('three')
  110
+    end
  111
+
  112
+    after_count = Project.count
  113
+    assert_equal 3, after_count - before_count
  114
+  end
  115
+
  116
+  private
  117
+
  118
+  def add_record(name)
  119
+    ActiveRecord::Base.connection_pool.with_connection { Project.create! :name => name }
  120
+  end
92 121
 end unless %w(FrontBase).include? ActiveRecord::Base.connection.adapter_name
93 122
 
94 123
 class AllowConcurrencyDeprecatedTest < ActiveRecord::TestCase

0 notes on commit 5501b99

coderrr

this line should be changed to ActiveRecord::Base.connection_pool.rollback_db_transaction. The raise will just kill the thread so it does nothing, we want to actually make that call to the db.

Please sign in to comment.
Something went wrong with that request. Please try again.