StructuredEventSubscriber の debug_only がサブクラス間で漏れないように修正

ActiveSupport::StructuredEventSubscriber の debug_only がサブクラス間で共有されていた配列に依存し、意図しないメソッドが全体に漏れるバグを、配列の破壊的操作から非破壊的再代入へ変更することで解消しました。

背景

ActiveSupport::StructuredEventSubscriber は debug_methods を class_attribute で定義し、デフォルトは空配列 [] です。debug_only が呼び出されるたびに self.debug_methods << method が実行されるため、同一オブジェクトの配列がすべてのサブクラスで共有され、メソッド名が汚染されてしまいます。

再現例として以下のコードを実行すると、二つのサブクラスが互いのメソッドを取得し、基底クラスまで汚染された配列が残ります。

sx = Class.new(ActiveSupport::StructuredEventSubscriber) { def foo(e); end; debug_only :foo }
sy = Class.new(ActiveSupport::StructuredEventSubscriber) { def bar(e); end; debug_only :bar }

sx.debug_methods      # => [:foo, :bar]   (期待は [:foo])
sy.debug_methods      # => [:foo, :bar]   (期待は [:bar])
ActiveSupport::StructuredEventSubscriber.debug_methods # => [:foo, :bar] (基底も汚染)

この状態では、フレームワークが内部で debug_only に登録したメソッド（例: :sql, :render_template）が、ユーザー実装のサブクラスにまで影響し、本番環境で本来通知すべきイベントが抑止される危険があります。

技術的な変更

debug_only の実装を 破壊的変更 から 非破壊的再代入 に置き換えるだけで、サブクラスごとに独立した配列が生成されるようになりました。

@@ -48,7 +48,7 @@ def set_silenced_events
         end

         def debug_only(method)
-          self.debug_methods << method
+          self.debug_methods += [method]
           set_silenced_events
         end
     end

self.debug_methods += [method] は class_attribute の書き込みメソッドを通して新しい配列を代入するため、デフォルト配列がコピーされ、各サブクラスが独自の debug_methods を保持します。これにより、元の共有オブジェクトは変更されず、汚染は起きません。

テストも同様に修正され、サブクラス間でのリークを検証する新しいテスト test_debug_only_does_not_leak_across_subclasses が追加されました。また、既存テスト test_debug_only_methods ではサブスクライバのアタッチ先を実際のサブクラス TestSubscriber に変更し、リークが無い前提で正しく動作することを確認しています。

@@
   teardown do
     ActiveSupport.event_reporter.debug_mode = @old_debug_mode
+    TestSubscriber.detach_from :test
+    ActiveSupport::StructuredEventSubscriber.detach_from :test
   end
@@
-    ActiveSupport::StructuredEventSubscriber.attach_to :test, @subscriber
+    TestSubscriber.attach_to :test, @subscriber
@@
+  def test_debug_only_does_not_leak_across_subclasses
+    base_methods = ActiveSupport::StructuredEventSubscriber.debug_methods.dup
+
+    subscriber_a = Class.new(ActiveSupport::StructuredEventSubscriber) do
+      def foo(event); end
+      debug_only :foo
+    end
+
+    subscriber_b = Class.new(ActiveSupport::StructuredEventSubscriber) do
+      def bar(event); end
+      debug_only :bar
+    end
+
+    assert_equal [:foo], subscriber_a.debug_methods
+    assert_equal [:bar], subscriber_b.debug_methods
+    assert_equal base_methods, ActiveSupport::StructuredEventSubscriber.debug_methods
+  end

この変更はコードサイズが僅か1行の差分で済み、既存の API 互換性を保持しつつバグを根本的に排除します。

設計判断

非破壊的再代入 を選択したのは、class_attribute が内部でコピーを返す特性を活かし、個別の配列インスタンスを自動的に生成させるためです。LogSubscriber.subscribe_log_level でも同様に self.log_levels = ... と再代入している点と同調し、コードベース全体で一貫した設計パターンを維持しています。

このアプローチは 後方互換性 を損なわず、debug_only を呼び出す既存のサブクラスはそのまま動作します。配列をその場で << する破壊的操作は削除され、将来的に同様の問題が別属性で再発するリスクも低減されます。

結果として、ActiveSupport のサブスクライバ機構はサブクラスごとに安全にデバッグ対象を限定でき、フレームワーク本体とユーザー実装の境界が明確になります。

まとめ

debug_only が内部配列を破壊的に変更していたために起きていたサブクラス間のメソッド漏れを、self.debug_methods += [method] という非破壊的再代入に置き換えるだけで解決しました。テストも拡充され、変更の安全性が検証されています。このシンプルな修正は、class_attribute の特性を正しく活用した設計判断の好例と言えるでしょう。