Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fix recursive mutex lock in connection_validator extension

The connection_validator extension was checking connections
for validity while holding the pool mutex.  This greatly
reduces the concurrency and can result in a recursive mutex
lock if the validation itself requires a connection (which
is true on some adapters if the validation SQL was not
cached).

Instead of wrapping next_available, wrap acquire, which is
not called with the mutex lock.  Manually do locking of the
necessary data structures when modifying them.
  • Loading branch information...
commit 40bcfcb9ff5b39c882c204792f1095e95300721f 1 parent ac3fe7e
@jeremyevans authored
View
25 lib/sequel/extensions/connection_validator.rb
@@ -59,6 +59,12 @@ def self.extended(pool)
@connection_timestamps ||= {}
@connection_validation_timeout = 3600
end
+
+ # Make sure the valid connection SQL query is precached,
+ # otherwise it's possible it will happen at runtime. While
+ # it should work correctly at runtime, it's better to avoid
+ # the possibility of failure altogether.
+ pool.db.send(:valid_connection_sql)
end
private
@@ -70,18 +76,23 @@ def checkin_connection(*)
conn
end
- # If an available connection is being checked out, and it has
- # has been idle for longer than the connection validation timeout,
+ # When acquiring a connection, if it has been
+ # idle for longer than the connection validation timeout,
# test the connection for validity. If it is not valid,
- # disconnect the connection, and retry with the next available
- # connection. If no available connections are valid, this will
- # return nil.
- def next_available(*)
+ # disconnect the connection, and retry with a new connection.
+ def acquire(*a)
begin
if (conn = super) &&
- (t = @connection_timestamps.delete(conn)) &&
+ (t = sync{@connection_timestamps.delete(conn)}) &&
Time.now - t > @connection_validation_timeout &&
!db.valid_connection?(conn)
+
+ if pool_type == :sharded_threaded
+ sync{allocated(a.last).delete(Thread.current)}
+ else
+ sync{@allocated.delete(Thread.current)}
+ end
+
db.disconnect_connection(conn)
raise Retry
end
View
7 spec/extensions/connection_validator_spec.rb
@@ -94,6 +94,13 @@ def connect(server)
@db.pool.instance_variable_get(:@connection_timestamps).should_not have_key(c1)
@db.pool.instance_variable_get(:@connection_timestamps).should have_key(c2)
end
+
+ it "should handle case where determining validity requires a connection" do
+ @db.meta_def(:valid_connection?){|c| synchronize{}; true}
+ @db.pool.connection_validation_timeout = -1
+ c1 = @db.synchronize{|c| c}
+ @db.synchronize{|c| c}.should equal(c1)
+ end
end
describe "Sequel::ConnectionValidator with threaded pool" do
Please sign in to comment.
Something went wrong with that request. Please try again.