Skip to content

Commit 65fd300

Browse files
authored
Merge pull request rails#53799 from byroot/mysql2-needless-affected-rows
Workaround for mysql2 prepared statements flaky test
2 parents a01b093 + b3c866f commit 65fd300

File tree

1 file changed

+33
-5
lines changed

1 file changed

+33
-5
lines changed

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

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,12 @@ def perform_query(raw_connection, sql, binds, type_casted_binds, prepare:, notif
5151
result = if binds.nil? || binds.empty?
5252
ActiveSupport::Dependencies.interlock.permit_concurrent_loads do
5353
result = raw_connection.query(sql)
54-
@affected_rows_before_warnings = raw_connection.affected_rows
54+
# Ref: https://github.com/brianmario/mysql2/pull/1383
55+
# As of mysql2 0.5.6 `#affected_rows` might raise Mysql2::Error if a prepared statement
56+
# from that same connection was GCed while `#query` released the GVL.
57+
# By avoiding to call `#affected_rows` when we have a result, we reduce the likeliness
58+
# of hitting the bug.
59+
@affected_rows_before_warnings = result&.size || raw_connection.affected_rows
5560
result
5661
end
5762
result
@@ -66,6 +71,12 @@ def perform_query(raw_connection, sql, binds, type_casted_binds, prepare:, notif
6671
ActiveSupport::Dependencies.interlock.permit_concurrent_loads do
6772
result = stmt.execute(*type_casted_binds)
6873
@affected_rows_before_warnings = stmt.affected_rows
74+
75+
# Ref: https://github.com/brianmario/mysql2/pull/1383
76+
# by eagerly closing uncached prepared statements, we also reduce the chances of
77+
# that bug happening. It can still happen if `#execute` is used as we have no callback
78+
# to eagerly close the statement.
79+
result.instance_variable_set(:@_ar_stmt_to_close, stmt) if result && !prepare
6980
result
7081
end
7182
rescue ::Mysql2::Error
@@ -97,17 +108,34 @@ def perform_query(raw_connection, sql, binds, type_casted_binds, prepare:, notif
97108
end
98109
end
99110

100-
def cast_result(result)
101-
if result.nil? || result.fields.empty?
111+
def cast_result(raw_result)
112+
return ActiveRecord::Result.empty if raw_result.nil?
113+
114+
fields = raw_result.fields
115+
116+
result = if fields.empty?
102117
ActiveRecord::Result.empty
103118
else
104-
ActiveRecord::Result.new(result.fields, result.to_a)
119+
ActiveRecord::Result.new(fields, raw_result.to_a)
105120
end
121+
122+
free_raw_result(raw_result)
123+
124+
result
106125
end
107126

108-
def affected_rows(result)
127+
def affected_rows(raw_result)
128+
free_raw_result(raw_result) if raw_result
129+
109130
@affected_rows_before_warnings
110131
end
132+
133+
def free_raw_result(raw_result)
134+
raw_result.free
135+
if stmt = raw_result.instance_variable_get(:@_ar_stmt_to_close)
136+
stmt.close
137+
end
138+
end
111139
end
112140
end
113141
end

0 commit comments

Comments
 (0)