From 1724e0d46ed21345cdac58548b37cf9cf23e547b Mon Sep 17 00:00:00 2001 From: Ryan Wallace Date: Fri, 23 Oct 2026 09:47:59 -0700 Subject: [PATCH 1/2] Pin the transaction strategy's connection on Rails 7.2+ Rails' executor releases a thread's connection when its transaction is not joinable, so an inline job or a request in a request spec hands the test's connection back to the pool. If another thread (the pool reaper's keepalive, for one) checks it out before the test queries again, the test leases a different connection that can't see its uncommitted records. Pin the connection the way ActiveRecord::TestFixtures does, and add a :lock_threads option matching Rails' lock_threads. --- CHANGELOG.md | 2 + README.md | 10 +++- .../active_record/transaction.rb | 41 ++++++++++++-- .../active_record/transaction_spec.rb | 56 +++++++++++++++++++ 4 files changed, 104 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1acecdce..8b907d5d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ # Development (unreleased) +* Pin the transaction strategy's connection on Rails 7.2+, as Rails' transactional tests do, so an executor wrap can't release it mid-test; add the `:lock_threads` option + ## v2.2.2 2025-07-30 * Release database connections after cleaning: https://github.com/DatabaseCleaner/database_cleaner-active_record/pull/122 diff --git a/README.md b/README.md index 3d7d49c3..1bd47fb3 100644 --- a/README.md +++ b/README.md @@ -43,7 +43,15 @@ Some people report much faster speeds with `:deletion` while others say `:trunca ## Strategy configuration options -The transaction strategy accepts no options. +On Rails 7.2 and later the transaction strategy pins the connection, as Rails' own transactional tests do: it stays leased while the test runs, even across executor wraps that release connections (such as `perform_enqueued_jobs` or a request in a request spec), and every thread uses it, so all of them see the test's uncommitted data. + +The transaction strategy accepts the following option: + +* `:lock_threads` (Rails 7.2+, default `true`) makes threads take turns on the shared connection, like Rails' `lock_threads`. Set it to `false` only if no two threads query at the same time. + + ```ruby + DatabaseCleaner[:active_record].strategy = DatabaseCleaner::ActiveRecord::Transaction.new(lock_threads: false) + ``` The truncation and deletion strategies may accept the following options: diff --git a/lib/database_cleaner/active_record/transaction.rb b/lib/database_cleaner/active_record/transaction.rb index c7391f0d..f881a002 100644 --- a/lib/database_cleaner/active_record/transaction.rb +++ b/lib/database_cleaner/active_record/transaction.rb @@ -1,13 +1,26 @@ module DatabaseCleaner module ActiveRecord class Transaction < Base + def initialize(opts={}) + if !opts.empty? && !(opts.keys - [:lock_threads]).empty? + raise ArgumentError, "The only valid option is :lock_threads. You specified #{opts.keys.join(',')}." + end + + @lock_threads = opts.has_key?(:lock_threads) ? !!opts[:lock_threads] : true + end + def start - connection = if ::ActiveRecord.version >= Gem::Version.new("7.2") - connection_class.lease_connection - else - connection_class.connection + if pin_connection? + # Pin the connection the way Rails' transactional tests do, so it stays leased (and its + # transaction visible) even when an executor wrap releases connections, and so other + # threads share it instead of checking out one that can't see the uncommitted data. + connection_class.connection_pool.pin_connection!(@lock_threads) + @pinned_depth = pinned_depth + 1 + return end + connection = connection_class.connection + # Hack to make sure that the connection is properly set up before cleaning connection.transaction {} @@ -16,6 +29,16 @@ def start def clean + if pin_connection? + pool = connection_class.connection_pool + while pinned_depth > 0 + pool.unpin_connection! + @pinned_depth -= 1 + end + pool.release_connection + return + end + connection_class.connection_pool.connections.each do |connection| connection.lock.synchronize do next unless connection.open_transactions > 0 @@ -25,6 +48,16 @@ def clean connection_class.connection_pool.release_connection end + + private + + def pin_connection? + ::ActiveRecord.version >= Gem::Version.new("7.2") + end + + def pinned_depth + @pinned_depth || 0 + end end end end diff --git a/spec/database_cleaner/active_record/transaction_spec.rb b/spec/database_cleaner/active_record/transaction_spec.rb index 4def03db..d12697e0 100644 --- a/spec/database_cleaner/active_record/transaction_spec.rb +++ b/spec/database_cleaner/active_record/transaction_spec.rb @@ -4,6 +4,16 @@ RSpec.describe DatabaseCleaner::ActiveRecord::Transaction do subject(:strategy) { described_class.new } + describe "options" do + it "accepts :lock_threads" do + expect { described_class.new(lock_threads: false) }.to_not raise_error + end + + it "rejects unknown options" do + expect { described_class.new(only: []) }.to raise_error(ArgumentError) + end + end + DatabaseHelper.with_all_dbs do |helper| context "using a #{helper.db} connection" do around do |example| @@ -56,6 +66,52 @@ end end + if ActiveRecord.version >= Gem::Version.new("7.2") + describe "connection pinning" do + let(:pool) { ActiveRecord::Base.connection_pool } + + after { strategy.clean } + + it "keeps the uncommitted records visible after an executor wrap releases connections" do + strategy.start + User.create! + + ActiveRecord::ConnectionAdapters::ConnectionPool::ExecutorHooks.complete(nil) + + # Another thread (e.g. the pool reaper) checks out the released connection meanwhile + leased, done = Queue.new, Queue.new + holder = Thread.new do + pool.lease_connection + leased << true + done.pop + pool.release_connection + end + leased.pop + + expect(User.count).to eq 1 + ensure + done << true + holder.join + end + + it "shares the connection with other threads" do + strategy.start + User.create! + + expect(Thread.new { User.count }.value).to eq 1 + end + + it "unpins the connection on #clean" do + strategy.start + connection = pool.lease_connection + strategy.clean + + expect(connection.transaction_open?).to be false + expect(pool.lease_connection.transaction_open?).to be false + end + end + end + describe "#cleaning" do context "with records" do it "should clean all tables" do From 87440f62695bc6f83b640e9c960cb7701ab07a21 Mon Sep 17 00:00:00 2001 From: Ryan Wallace Date: Mon, 26 Oct 2026 11:08:33 -0700 Subject: [PATCH 2/2] Open the pinned connection's transaction on Rails edge Rails main no longer begins a transaction in pin_connection! or rolls it back in unpin_connection!; ActiveRecord::TestFixtures now does both itself. Begin and roll back the transaction when pinning didn't, so the strategy works on 7.2-8.1 and on edge. Check the new specs' records by id rather than by table count, so rows left behind on sqlite3 by other examples don't fail them. --- .../active_record/transaction.rb | 20 +++++++++++++------ .../active_record/transaction_spec.rb | 8 ++++---- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/lib/database_cleaner/active_record/transaction.rb b/lib/database_cleaner/active_record/transaction.rb index f881a002..c8009846 100644 --- a/lib/database_cleaner/active_record/transaction.rb +++ b/lib/database_cleaner/active_record/transaction.rb @@ -14,8 +14,13 @@ def start # Pin the connection the way Rails' transactional tests do, so it stays leased (and its # transaction visible) even when an executor wrap releases connections, and so other # threads share it instead of checking out one that can't see the uncommitted data. - connection_class.connection_pool.pin_connection!(@lock_threads) - @pinned_depth = pinned_depth + 1 + pool = connection_class.connection_pool + pool.pin_connection!(@lock_threads) + connection = pool.lease_connection + # Rails 7.2-8.1 open the transaction in pin_connection!; later versions leave it to the caller. + began_transaction = !connection.transaction_open? + connection.begin_transaction(joinable: false, _lazy: false) if began_transaction + pins << began_transaction return end @@ -31,9 +36,12 @@ def start def clean if pin_connection? pool = connection_class.connection_pool - while pinned_depth > 0 + until pins.empty? + if pins.pop + connection = pool.lease_connection + connection.rollback_transaction if connection.transaction_open? + end pool.unpin_connection! - @pinned_depth -= 1 end pool.release_connection return @@ -55,8 +63,8 @@ def pin_connection? ::ActiveRecord.version >= Gem::Version.new("7.2") end - def pinned_depth - @pinned_depth || 0 + def pins + @pins ||= [] end end end diff --git a/spec/database_cleaner/active_record/transaction_spec.rb b/spec/database_cleaner/active_record/transaction_spec.rb index d12697e0..cd10b941 100644 --- a/spec/database_cleaner/active_record/transaction_spec.rb +++ b/spec/database_cleaner/active_record/transaction_spec.rb @@ -74,7 +74,7 @@ it "keeps the uncommitted records visible after an executor wrap releases connections" do strategy.start - User.create! + user = User.create! ActiveRecord::ConnectionAdapters::ConnectionPool::ExecutorHooks.complete(nil) @@ -88,7 +88,7 @@ end leased.pop - expect(User.count).to eq 1 + expect(User.exists?(user.id)).to be true ensure done << true holder.join @@ -96,9 +96,9 @@ it "shares the connection with other threads" do strategy.start - User.create! + user = User.create! - expect(Thread.new { User.count }.value).to eq 1 + expect(Thread.new { User.exists?(user.id) }.value).to be true end it "unpins the connection on #clean" do