Skip to content
This repository
Browse code

make sure connections returned after close are marked as in_use

  • Loading branch information...
commit 5f26ce699f9e695c434cbff20626a9ff4d3114e4 1 parent da0595d
Aaron Patterson authored
16  activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -269,11 +269,27 @@ def checkin(conn)
269 269
             conn.expire
270 270
             @queue.signal
271 271
           end
  272
+
  273
+          release conn
272 274
         end
273 275
       end
274 276
 
275 277
       private
276 278
 
  279
+      def release(conn)
  280
+        thread_id = nil
  281
+
  282
+        if @reserved_connections[current_connection_id] == conn
  283
+          thread_id = current_connection_id
  284
+        else
  285
+          thread_id = @reserved_connections.keys.find { |k|
  286
+            @reserved_connections[k] == conn
  287
+          }
  288
+        end
  289
+
  290
+        @reserved_connections.delete thread_id if thread_id
  291
+      end
  292
+
277 293
       def new_connection
278 294
         ActiveRecord::Base.send(spec.adapter_method, spec.config)
279 295
       end
10  activerecord/test/cases/connection_pool_test.rb
@@ -31,6 +31,16 @@ def active_connections(pool)
31 31
         pool.connections.find_all(&:in_use?)
32 32
       end
33 33
 
  34
+      def test_checkout_after_close
  35
+        connection = pool.connection
  36
+        assert connection.in_use?
  37
+
  38
+        connection.close
  39
+        assert !connection.in_use?
  40
+
  41
+        assert pool.connection.in_use?
  42
+      end
  43
+
34 44
       def test_released_connection_moves_between_threads
35 45
         thread_conn = nil
36 46
 

3 notes on commit 5f26ce6

Jonathan Rochkind

Awesome. Am hoping this helps solve a related problem I am trying to investigate in my app, causing it to require more connections in the pool than I think it ought to, since rails 3.0 even.

However, you now have two similar but different methods with similar but different names: release(conn) and release_connection(conn). Can you maybe put some inline comments in the new release explaining how it's different than release_connection

Alternately, do they need to be two different methods? The new release seems to possibly do a superset of what the old release_connection did -- should they just be combined?

Aaron Patterson
Owner

release_connection actually takes the thread id associated with the connection. I think it's meant to be called with no arguments. release is meant to be called with a connection, and it's private. We could probably combine release_connection and release, but I wanted to make the smallest change possible. Fix bug in one commit, refactor later. :-)

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