From a6c01974b16b0a9c76101dfac529e5ca1ee8007b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A9=20Dupuis?= Date: Sat, 23 Mar 2024 14:09:17 -0700 Subject: [PATCH] Fix a crash when an Executor wrapped fork exit. Fix #51298 forking inside an Execution Wrapper crashes when running the completion callbacks. Rails 7.0 was not Execution wrapping the `runner` command. Rails 7.2 changed the definition of `active_connection?`, the new definition doesn't contain the bug. Therefore forking inside a script intended to be run with the `runner` command on 7.1 crashes. (see #51298) --- activerecord/lib/active_record/query_cache.rb | 2 +- activerecord/test/cases/query_cache_test.rb | 31 +++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/query_cache.rb b/activerecord/lib/active_record/query_cache.rb index 08db278880fe1..3765986567fef 100644 --- a/activerecord/lib/active_record/query_cache.rb +++ b/activerecord/lib/active_record/query_cache.rb @@ -30,7 +30,7 @@ def self.run end def self.complete(pools) - pools.each { |pool| pool.disable_query_cache! } + pools.each { |pool| pool.disable_query_cache! unless pool.discarded? } ActiveRecord::Base.connection_handler.each_connection_pool do |pool| pool.release_connection if pool.active_connection? && !pool.connection.transaction_open? diff --git a/activerecord/test/cases/query_cache_test.rb b/activerecord/test/cases/query_cache_test.rb index 43a4031405888..91b8842cb05ae 100644 --- a/activerecord/test/cases/query_cache_test.rb +++ b/activerecord/test/cases/query_cache_test.rb @@ -650,6 +650,37 @@ def test_clear_query_cache_is_called_on_all_connections ActiveRecord::Base.connection_pool.lock_thread = false end + test "query cache callbacks exit gracefully from a fork" do + # Regression test for #51298 + pid = nil + status = nil + + # Run forking Rack app + middleware { |env| + pid = fork + _, status = Process.wait2(pid) if pid.present? + [200, {}, nil] + }.call({}) + + + # Happy path, the fork exit 0 and the main process + # asserts on the exit status of the fork + if pid.nil? + exit 0 + else + assert_equal 0, status.exitstatus + end + rescue + # Unhappy path, the fork exit 1. + # The main process should not reach here, but in case + # something's wrong we need to re-raise the error. + if pid.nil? + exit 1 + else + raise + end + end + private def with_temporary_connection_pool(&block) pool_config = ActiveRecord::Base.connection.pool.pool_config