Skip to content

Commit 548bb83

Browse files
Don't query adapter in ensure blocks when connection is closed
Trilogy closes its connection when it encounters a timeout. Active Record doesn't always account for this, and can query the adapter in ensure blocks of certain adapter methods. Instead, we need to preface these queries with active checks to make sure we don't swallow the timeout and raise an adapter error instead. Co-authored-by: Adrianna Chang <[email protected]>
1 parent a534bac commit 548bb83

File tree

4 files changed

+38
-3
lines changed

4 files changed

+38
-3
lines changed

activerecord/lib/active_record/connection_adapters/abstract/transaction.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -350,7 +350,7 @@ def restart
350350

351351
def rollback
352352
unless @state.invalidated?
353-
connection.rollback_to_savepoint(savepoint_name) if materialized?
353+
connection.rollback_to_savepoint(savepoint_name) if materialized? && connection.active?
354354
end
355355
@state.rollback!
356356
@instrumenter.finish(:rollback) if materialized?

activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ def disable_referential_integrity # :nodoc:
218218
update("SET FOREIGN_KEY_CHECKS = 0")
219219
yield
220220
ensure
221-
update("SET FOREIGN_KEY_CHECKS = #{old}")
221+
update("SET FOREIGN_KEY_CHECKS = #{old}") if active?
222222
end
223223
end
224224

activerecord/lib/active_record/connection_adapters/trilogy/database_statements.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ def with_multi_statements
9696

9797
yield
9898
ensure
99-
conn.set_server_option(::Trilogy::SET_SERVER_MULTI_STATEMENTS_OFF)
99+
conn.set_server_option(::Trilogy::SET_SERVER_MULTI_STATEMENTS_OFF) if active?
100100
end
101101
end
102102
end

activerecord/test/cases/adapters/trilogy/trilogy_adapter_test.rb

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
require "support/ddl_helper"
55
require "models/book"
66
require "models/post"
7+
require "timeout"
78

89
class TrilogyAdapterTest < ActiveRecord::TrilogyTestCase
910
setup do
@@ -17,6 +18,40 @@ class TrilogyAdapterTest < ActiveRecord::TrilogyTestCase
1718
assert_kind_of ActiveRecord::ConnectionAdapters::NullPool, error.connection_pool
1819
end
1920

21+
test "timeout in transaction doesnt query closed connection" do
22+
assert_raises(Timeout::Error) do
23+
Timeout.timeout(0.1) do
24+
@conn.transaction do
25+
@conn.execute("SELECT SLEEP(1)")
26+
end
27+
end
28+
end
29+
end
30+
31+
test "timeout in fixture set insertion doesnt query closed connection" do
32+
fixtures = [
33+
["traffic_lights", [
34+
{ "location" => "US", "state" => ["NY"], "long_state" => ["a"] },
35+
]]
36+
] * 1000
37+
38+
assert_raises(Timeout::Error) do
39+
Timeout.timeout(0.1) do
40+
@conn.insert_fixtures_set(fixtures)
41+
end
42+
end
43+
end
44+
45+
test "timeout without referential integrity doesnt query closed connection" do
46+
assert_raises(Timeout::Error) do
47+
Timeout.timeout(0.1) do
48+
@conn.disable_referential_integrity do
49+
@conn.execute("SELECT SLEEP(1)")
50+
end
51+
end
52+
end
53+
end
54+
2055
test "#explain for one query" do
2156
explain = @conn.explain("select * from posts")
2257
assert_match %(possible_keys), explain

0 commit comments

Comments
 (0)