Skip to content

Commit bcb2fd3

Browse files
authored
Merge pull request rails#53881 from byroot/adapters-consistent-interlock
Consistently release the autoloading interlock from all adapters
2 parents 2497eba + 23d1e07 commit bcb2fd3

File tree

5 files changed

+21
-23
lines changed

5 files changed

+21
-23
lines changed

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -553,7 +553,11 @@ def raw_execute(sql, name = nil, binds = [], prepare: false, async: false, allow
553553
type_casted_binds = type_casted_binds(binds)
554554
log(sql, name, binds, type_casted_binds, async: async) do |notification_payload|
555555
with_raw_connection(allow_retry: allow_retry, materialize_transactions: materialize_transactions) do |conn|
556-
perform_query(conn, sql, binds, type_casted_binds, prepare: prepare, notification_payload: notification_payload, batch: batch)
556+
result = ActiveSupport::Dependencies.interlock.permit_concurrent_loads do
557+
perform_query(conn, sql, binds, type_casted_binds, prepare: prepare, notification_payload: notification_payload, batch: batch)
558+
end
559+
handle_warnings(result, sql)
560+
result
557561
end
558562
end
559563
end
@@ -562,6 +566,9 @@ def perform_query(raw_connection, sql, binds, type_casted_binds, prepare:, notif
562566
raise NotImplementedError
563567
end
564568

569+
def handle_warnings(raw_result, sql)
570+
end
571+
565572
# Receive a native adapter result object and returns an ActiveRecord::Result object.
566573
def cast_result(raw_result)
567574
raise NotImplementedError

activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -759,7 +759,7 @@ def extended_type_map_key
759759
end
760760
end
761761

762-
def handle_warnings(sql)
762+
def handle_warnings(_initial_result, sql)
763763
return if ActiveRecord.db_warnings_action.nil? || @raw_connection.warning_count == 0
764764

765765
warning_count = @raw_connection.warning_count

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

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -50,22 +50,18 @@ def perform_query(raw_connection, sql, binds, type_casted_binds, prepare:, notif
5050

5151
result = nil
5252
if binds.nil? || binds.empty?
53-
ActiveSupport::Dependencies.interlock.permit_concurrent_loads do
54-
result = raw_connection.query(sql)
55-
# Ref: https://github.com/brianmario/mysql2/pull/1383
56-
# As of mysql2 0.5.6 `#affected_rows` might raise Mysql2::Error if a prepared statement
57-
# from that same connection was GCed while `#query` released the GVL.
58-
# By avoiding to call `#affected_rows` when we have a result, we reduce the likeliness
59-
# of hitting the bug.
60-
@affected_rows_before_warnings = result&.size || raw_connection.affected_rows
61-
end
53+
result = raw_connection.query(sql)
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
6260
elsif prepare
6361
stmt = @statements[sql] ||= raw_connection.prepare(sql)
6462
begin
65-
ActiveSupport::Dependencies.interlock.permit_concurrent_loads do
66-
result = stmt.execute(*type_casted_binds)
67-
@affected_rows_before_warnings = stmt.affected_rows
68-
end
63+
result = stmt.execute(*type_casted_binds)
64+
@affected_rows_before_warnings = stmt.affected_rows
6965
rescue ::Mysql2::Error
7066
@statements.delete(sql)
7167
raise
@@ -74,10 +70,8 @@ def perform_query(raw_connection, sql, binds, type_casted_binds, prepare:, notif
7470
stmt = raw_connection.prepare(sql)
7571

7672
begin
77-
ActiveSupport::Dependencies.interlock.permit_concurrent_loads do
78-
result = stmt.execute(*type_casted_binds)
79-
@affected_rows_before_warnings = stmt.affected_rows
80-
end
73+
result = stmt.execute(*type_casted_binds)
74+
@affected_rows_before_warnings = stmt.affected_rows
8175

8276
# Ref: https://github.com/brianmario/mysql2/pull/1383
8377
# by eagerly closing uncached prepared statements, we also reduce the chances of
@@ -100,7 +94,6 @@ def perform_query(raw_connection, sql, binds, type_casted_binds, prepare:, notif
10094
raw_connection.abandon_results!
10195

10296
verified!
103-
handle_warnings(sql)
10497
result
10598
ensure
10699
if reset_multi_statement && active?

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,6 @@ def perform_query(raw_connection, sql, binds, type_casted_binds, prepare:, notif
163163
end
164164

165165
verified!
166-
handle_warnings(result)
167166

168167
notification_payload[:affected_rows] = result.cmd_tuples
169168
notification_payload[:row_count] = result.count
@@ -215,7 +214,7 @@ def suppress_composite_primary_key(pk)
215214
pk unless pk.is_a?(Array)
216215
end
217216

218-
def handle_warnings(sql)
217+
def handle_warnings(result, sql)
219218
@notice_receiver_sql_warnings.each do |warning|
220219
next if warning_ignored?(warning)
221220

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ def perform_query(raw_connection, sql, binds, type_casted_binds, prepare:, notif
2929
raw_connection.next_result
3030
end
3131
verified!
32-
handle_warnings(sql)
3332

3433
notification_payload[:affected_rows] = result.affected_rows
3534
notification_payload[:row_count] = result.count

0 commit comments

Comments
 (0)