Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

QueryCache/ConnectionPool thread fix (was #1670) #3243

Merged
merged 1 commit into from

3 participants

@mjtko

(This was originally pull request #1670, but that was against 3-1-stable - this pull request is based against master instead).

Under thin, QueryCache is leaking connections.

This is caused by the fact that the thread that runs the QueryCache::BodyProxy#close method is not the same as the thread that executes QueryCache#call. A connection is checked out within QueryCache#call; this connection becomes orphaned, while a new connection is checked out within #close when theclear_query_cache call is made.

This patch prevents the connection checked out within QueryCache#call from being orphaned by creating an identifier that is stored within a thread local and used within ConnectionPool to uniquely identify the connection. This identifier is passed to BodyProxy at construction time and pushed into the thread that ultimately becomes responsible for the clear_query_cache call.

The connection is subsequently checked back in as expected at the end of the request lifecycle, plugging the leak.

@mjtko mjtko use thread locals and an instance variable within QueryCache#BodyProx…
…y to maintain appropriate linkage with AR database connection across threads
f41b58d
@josevalim
Owner

/cc @tenderlove please take a look at this. it is important and i think we should merge this into 3-1-stable for heroku.

@spastorino spastorino merged commit ea341b8 into rails:master
@spastorino
Owner

I will merge it to 3-1-stable after 3.1.1 release

@spastorino
Owner

Done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 7, 2011
  1. @mjtko

    use thread locals and an instance variable within QueryCache#BodyProx…

    mjtko authored
    …y to maintain appropriate linkage with AR database connection across threads
This page is out of date. Refresh to see the latest.
View
2  activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -314,7 +314,7 @@ def new_connection
end
def current_connection_id #:nodoc:
- Thread.current.object_id
+ ActiveRecord::Base.connection_id ||= Thread.current.object_id
end
def checkout_new_connection
View
8 activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
@@ -115,6 +115,14 @@ def connection
retrieve_connection
end
+ def connection_id
+ Thread.current['ActiveRecord::Base.connection_id']
+ end
+
+ def connection_id=(connection_id)
+ Thread.current['ActiveRecord::Base.connection_id'] = connection_id
+ end
+
# Returns the configuration of the associated connection as a hash:
#
# ActiveRecord::Base.connection_config
View
6 activerecord/lib/active_record/query_cache.rb
@@ -28,9 +28,10 @@ def initialize(app)
end
class BodyProxy # :nodoc:
- def initialize(original_cache_value, target)
+ def initialize(original_cache_value, target, connection_id)
@original_cache_value = original_cache_value
@target = target
+ @connection_id = connection_id
end
def method_missing(method_sym, *arguments, &block)
@@ -48,6 +49,7 @@ def each(&block)
def close
@target.close if @target.respond_to?(:close)
ensure
+ ActiveRecord::Base.connection_id = @connection_id
ActiveRecord::Base.connection.clear_query_cache
unless @original_cache_value
ActiveRecord::Base.connection.disable_query_cache!
@@ -60,7 +62,7 @@ def call(env)
ActiveRecord::Base.connection.enable_query_cache!
status, headers, body = @app.call(env)
- [status, headers, BodyProxy.new(old, body)]
+ [status, headers, BodyProxy.new(old, body, ActiveRecord::Base.connection_id)]
rescue Exception => e
ActiveRecord::Base.connection.clear_query_cache
unless old
Something went wrong with that request. Please try again.