テスト teardown 時のプール Reaper との競合による誤検知を修正

2026年05月18日 15:59 JST rails/rails

https://github.com/rails/rails/pull/57370

ActiveRecord::TestCase#check_connection_leaks が ConnectionPool::Reaper のメンテナンス処理と競合し、正常な接続を「リーク」と誤報告していた問題が修正されました。reaper_lock で排他制御することで、teardown がメンテナンス完了を待機してから接続の使用状態を判定するようになります。

背景

check_connection_leaks は teardown 時にプール内の接続をスキャンし、バックグラウンドスレッドが接続を保持したままテストを終えていないかを検査するメソッドです。pool.connections を走査して conn.in_use? が真であれば「リーク」と判断し、所有スレッドを kill します。

一方、ConnectionPool::Reaper は定期的にメンテナンス（タイムアウト接続の回収等）を行う際、reaper_lock を取得した状態で一時的に接続を保持します。teardown のリークチェックがこの期間中に pool.connections を参照した場合、Reaper が保持している接続を in_use? と見なし、AR Pool Reaper スレッドを kill してしまいます。テスト自体は何もリークしていないにもかかわらず、です。

この問題は #57367 の検証中に CI で発生した無関係な失敗として発覚し、報告されたリークオーナーが Reaper スレッドであったことで特定されました。

技術的な変更

activerecord/test/cases/test_case.rb の check_connection_leaks メソッド内で、プールの走査全体を pool.reaper_lock ブロックで包む変更が加えられました。

変更前:

pool.reap
pool.connections.each do |conn|
  if conn.in_use?
    if conn.owner != Fiber.current && conn.owner != Thread.current
      leaked_conn << [conn.owner, conn.owner.backtrace]
      conn.owner&.kill
    end
    conn.steal!
    pool.checkin(conn)
  end
end

変更後:

# Avoid racing the pool reaper while it is performing maintenance.
pool.reaper_lock do
  pool.reap
  pool.connections.each do |conn|
    if conn.in_use?
      if conn.owner != Fiber.current && conn.owner != Thread.current
        leaked_conn << [conn.owner, conn.owner.backtrace]
        conn.owner&.kill
      end
      conn.steal!
      pool.checkin(conn)
    end
  end
end

pool.reap と pool.connections.each の両方が reaper_lock ブロック内に移動しています。これにより teardown は Reaper が進行中のメンテナンスを完了させるまでロック取得を待ち、走査のタイミングで Reaper が接続を保持していない状態を保証します。

同時に、リグレッションテストとして activerecord/test/cases/test_case_test.rb が新規追加されました。テストは以下の手順で競合条件を再現します：

Thread.current.name = "AR Pool Reaper" で Reaper スレッドを模倣したスレッドを起動し、reaper_lock を取得して接続を保持する
pool.stub(:reaper_lock, ...) でリークチェックが reaper_lock に到達したことを CountDownLatch で検知する
リークチェック完了後に leaked_conn が空であることを確認する

このテストはパッチ適用前の origin/main では失敗し、適用後に通過することが確認されています。

設計判断

reaper_lock をリークチェック側で取得する という対称的なアプローチが採用されました。

リークチェックの目的は「テストが終了した後に、本来解放されているべき接続が保持されていないか」を検査することです。Reaper によるメンテナンス中の接続保持はその定義から外れますが、既存の実装はこの区別を行っていませんでした。reaper_lock を共有することで、Reaper がロックを保持している間はリークチェックが走らない（または逆に Reaper がロックを待つ）という相互排除が実現されます。

また、修正のスコープがテストインフラ（test/cases/test_case.rb）に限定されており、プロダクションコードへの変更を伴わない点も注目に値します。競合はテストのテアダウン処理固有の問題であり、修正もその層に閉じています。

まとめ

本PRは、reaper_lock という既存の排他制御機構をリークチェックと共有することで、最小限の変更で競合を解消しています。CI における断続的な誤検知を排除するとともに、Reaper スレッドが誤って kill されることでテスト環境が不安定になるリスクも取り除いた修正といえます。