From 442c6f16f10ea578e1a87717ce6da7f5716cad13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Thu, 11 Feb 2021 23:51:53 +0000 Subject: [PATCH] Merge PR #41356 Closes #41356 Closes #41034 --- activerecord/CHANGELOG.md | 7 +++++++ .../connection_adapters/postgresql_adapter.rb | 6 +----- .../adapters/postgresql/statement_pool_test.rb | 18 ++++++++++++++++++ 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index ebb71b529360d..c7909a2f5365f 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,10 @@ +* Increment postgres prepared statement counter before making a prepared statement, so if the statement is aborted + without Rails knowledge (e.g., if app gets kill -9d during long-running query or due to Rack::Timeout), app won't end + up in perpetual crash state for being inconsistent with Postgres. + + *wbharding*, *Martin Tepper* + + ## Rails 6.1.2.1 (February 10, 2021) ## * Fix possible DoS vector in PostgreSQL money type diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 54cbc9aaa12c0..4d97ad420af5e 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -227,11 +227,7 @@ def initialize(connection, max) end def next_key - "a#{@counter + 1}" - end - - def []=(sql, key) - super.tap { @counter += 1 } + "a#{@counter += 1}" end private diff --git a/activerecord/test/cases/adapters/postgresql/statement_pool_test.rb b/activerecord/test/cases/adapters/postgresql/statement_pool_test.rb index fef4b02b04e1d..6029630d890e6 100644 --- a/activerecord/test/cases/adapters/postgresql/statement_pool_test.rb +++ b/activerecord/test/cases/adapters/postgresql/statement_pool_test.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true require "cases/helper" +require "models/computer" +require "models/developer" module ActiveRecord module ConnectionAdapters @@ -16,6 +18,8 @@ def status end class StatementPoolTest < ActiveRecord::PostgreSQLTestCase + fixtures :developers + if Process.respond_to?(:fork) def test_cache_is_per_pid cache = StatementPool.new nil, 10 @@ -37,6 +41,20 @@ def test_dealloc_does_not_raise_on_inactive_connection cache["foo"] = "bar" assert_nothing_raised { cache.clear } end + + def test_prepared_statements_do_not_get_stuck_on_query_interruption + pg_connection = ActiveRecord::Base.connection.instance_variable_get(:@connection) + pg_connection.stub(:get_last_result, -> { raise "random error" }) do + assert_raises(RuntimeError) do + Developer.where(name: "David").last + end + + # without fix, this raises PG::DuplicatePstatement: ERROR: prepared statement "a3" already exists + assert_raises(RuntimeError) do + Developer.where(name: "David").last + end + end + end end end end