From b400e70a0073e8e36148eca5797f9424d2218dac Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Fri, 25 Oct 2019 15:53:03 -0700 Subject: [PATCH] Make AS::Deprecation.silence block thread-local Previouly `ActiveSupport::Deprecation.silence { ... }` would silence deprecations from all threads, which I think is undesirable in a mutli-threaded environment. This leaves `ActiveSupport::Deprecation.silenced=` as setting a global option, which might be a little confusing, but I think that these are used in different ways, and this best preserves how they are generally used. `silence {}` is used to silence a small section of code, `silenced=` is usually used to set a default in an initializer or test helper. I somewhat think that `ActiveSupport::Deprecation.silenced=` should be deprecated in favour of `ActiveSupport::Deprecation.behavior = :silence` --- .../lib/active_support/deprecation.rb | 1 + .../active_support/deprecation/reporting.rb | 13 +++++---- activesupport/test/deprecation_test.rb | 28 +++++++++++++++++++ 3 files changed, 36 insertions(+), 6 deletions(-) diff --git a/activesupport/lib/active_support/deprecation.rb b/activesupport/lib/active_support/deprecation.rb index 16db547016..5c743108dc 100644 --- a/activesupport/lib/active_support/deprecation.rb +++ b/activesupport/lib/active_support/deprecation.rb @@ -41,6 +41,7 @@ module ActiveSupport # By default, warnings are not silenced and debugging is off. self.silenced = false self.debug = false + @silenced_thread = Concurrent::ThreadLocalVar.new(false) end end end diff --git a/activesupport/lib/active_support/deprecation/reporting.rb b/activesupport/lib/active_support/deprecation/reporting.rb index 7075b5b869..b9748d0630 100644 --- a/activesupport/lib/active_support/deprecation/reporting.rb +++ b/activesupport/lib/active_support/deprecation/reporting.rb @@ -6,7 +6,7 @@ module ActiveSupport class Deprecation module Reporting # Whether to print a message (silent mode) - attr_accessor :silenced + attr_writer :silenced # Name of gem where method is deprecated attr_accessor :gem_name @@ -33,11 +33,12 @@ module ActiveSupport # ActiveSupport::Deprecation.warn('something broke!') # end # # => nil - def silence - old_silenced, @silenced = @silenced, true - yield - ensure - @silenced = old_silenced + def silence(&block) + @silenced_thread.bind(true, &block) + end + + def silenced + @silenced || @silenced_thread.value end def deprecation_warning(deprecated_method_name, message = nil, caller_backtrace = nil) diff --git a/activesupport/test/deprecation_test.rb b/activesupport/test/deprecation_test.rb index 9898a1d9a0..3c38083add 100644 --- a/activesupport/test/deprecation_test.rb +++ b/activesupport/test/deprecation_test.rb @@ -290,6 +290,34 @@ class DeprecationTest < ActiveSupport::TestCase ActiveSupport::Deprecation.silenced = false end + def test_silence_threaded + barrier = Concurrent::CyclicBarrier.new(2) + + th = Thread.new do + ActiveSupport::Deprecation.silence do + barrier.wait + barrier.wait + assert_not_deprecated { ActiveSupport::Deprecation.warn "abc" } + end + assert_deprecated("abc") { ActiveSupport::Deprecation.warn "abc" } + end + + barrier.wait + + assert_deprecated("abc") { ActiveSupport::Deprecation.warn "abc" } + + ActiveSupport::Deprecation.silence do + assert_not_deprecated { ActiveSupport::Deprecation.warn "abc" } + end + + assert_deprecated("abc") { ActiveSupport::Deprecation.warn "abc" } + + barrier.wait + th.join + ensure + th.kill + end + def test_deprecation_without_explanation assert_deprecated { @dtc.a } assert_deprecated { @dtc.b }