Skip to content

Commit

Permalink
Eagerly close Mysql2 prepared statements
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
byroot committed Dec 2, 2024
1 parent 28033d5 commit c078b4c
Showing 1 changed file with 27 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,12 @@ def perform_query(raw_connection, sql, binds, type_casted_binds, prepare:, notif
ActiveSupport::Dependencies.interlock.permit_concurrent_loads do
result = stmt.execute(*type_casted_binds)
@affected_rows_before_warnings = stmt.affected_rows

# Ref: https://github.com/brianmario/mysql2/pull/1383
# by eagerly closing uncached prepared statements, we also reduce the chances of
# that bug happening. It can still happen if `#execute` is used as we have no callback
# to eagerly close the statement.
result.instance_variable_set(:@_ar_stmt_to_close, stmt) if result && !prepare
result
end
rescue ::Mysql2::Error
Expand Down Expand Up @@ -102,17 +108,34 @@ def perform_query(raw_connection, sql, binds, type_casted_binds, prepare:, notif
end
end

def cast_result(result)
if result.nil? || result.fields.empty?
def cast_result(raw_result)
return ActiveRecord::Result.empty if raw_result.nil?

fields = raw_result.fields

result = if fields.empty?
ActiveRecord::Result.empty
else
ActiveRecord::Result.new(result.fields, result.to_a)
ActiveRecord::Result.new(fields, raw_result.to_a)
end

free_raw_result(raw_result)

result
end

def affected_rows(result)
def affected_rows(raw_result)
free_raw_result(raw_result) if raw_result

@affected_rows_before_warnings
end

def free_raw_result(raw_result)
raw_result.free
if stmt = raw_result.instance_variable_get(:@_ar_stmt_to_close)
stmt.close
end
end
end
end
end
Expand Down

0 comments on commit c078b4c

Please sign in to comment.