Merge pull request #51447 from Shopify/backtrace-cleaner-dup

Make ActiveSupport::BacktraceCleaner copy filters and silencers on dup and clone

activesupport/CHANGELOG.md

@@ -1,3 +1,10 @@
+*   Make ActiveSupport::BacktraceCleaner copy filters and silencers on dup and clone
+
+    Previously the copy would still share the internal silencers and filters array,
+    causing state to leak.
+
+    *Jean Boussier*
+
 *   Updating Astana with Western Kazakhstan TZInfo identifier
 
     *Damian Nelson*

activesupport/lib/active_support/backtrace_cleaner.rb

@@ -110,6 +110,11 @@ def remove_filters!
     private
       FORMATTED_GEMS_PATTERN = /\A[^\/]+ \([\w.]+\) /
 
+      def initialize_copy(_other)
+        @filters = @filters.dup
+        @silencers = @silencers.dup
+      end
+
       def add_gem_filter
         gems_paths = (Gem.path | [Gem.default_dir]).map { |p| Regexp.escape(p) }
         return if gems_paths.empty?

activesupport/test/backtrace_cleaner_test.rb

@@ -22,6 +22,14 @@ def setup
   test "backtrace should contain unaltered lines if they don't match a filter" do
     assert_equal "/my/other_prefix/my/class.rb", @bc.clean([ "/my/other_prefix/my/class.rb" ]).first
   end
+
+  test "#dup also copy filters" do
+    copy = @bc.dup
+    @bc.add_filter { |line| line.gsub("/other/prefix/", "") }
+
+    assert_equal "my/class.rb", @bc.clean(["/other/prefix/my/class.rb"]).first
+    assert_equal "/other/prefix/my/class.rb", copy.clean(["/other/prefix/my/class.rb"]).first
+  end
 end
 
 class BacktraceCleanerSilencerTest < ActiveSupport::TestCase
@@ -40,6 +48,14 @@ def setup
     @bc.remove_silencers!
     assert_equal ["/mongrel/stuff.rb"], @bc.clean(["/mongrel/stuff.rb"])
   end
+
+  test "#dup also copy silencers" do
+    copy = @bc.dup
+
+    @bc.add_silencer { |line| line.include?("puma") }
+    assert_equal [], @bc.clean(["/puma/stuff.rb"])
+    assert_equal ["/puma/stuff.rb"], copy.clean(["/puma/stuff.rb"])
+  end
 end
 
 class BacktraceCleanerMultipleSilencersTest < ActiveSupport::TestCase