Skip to content

Commit b3c866f

Browse files
committed
Eagerly close Mysql2 prepared statements
Ref: brianmario/mysql2#1383 If we eagerly close uncached statements, we drastically reduce the risk of one of them being GCed between `#query` and `#affected_rows`. The bug is still there thouhg, and can happen when `#execute` is used.
1 parent d45d186 commit b3c866f

File tree

1 file changed

+27
-4
lines changed

1 file changed

+27
-4
lines changed

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

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,12 @@ def perform_query(raw_connection, sql, binds, type_casted_binds, prepare:, notif
7171
ActiveSupport::Dependencies.interlock.permit_concurrent_loads do
7272
result = stmt.execute(*type_casted_binds)
7373
@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
7480
result
7581
end
7682
rescue ::Mysql2::Error
@@ -102,17 +108,34 @@ def perform_query(raw_connection, sql, binds, type_casted_binds, prepare:, notif
102108
end
103109
end
104110

105-
def cast_result(result)
106-
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?
107117
ActiveRecord::Result.empty
108118
else
109-
ActiveRecord::Result.new(result.fields, result.to_a)
119+
ActiveRecord::Result.new(fields, raw_result.to_a)
110120
end
121+
122+
free_raw_result(raw_result)
123+
124+
result
111125
end
112126

113-
def affected_rows(result)
127+
def affected_rows(raw_result)
128+
free_raw_result(raw_result) if raw_result
129+
114130
@affected_rows_before_warnings
115131
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
116139
end
117140
end
118141
end

0 commit comments

Comments
 (0)