Fix thread_mattr_accessor default option behavior

This makes the value supplied to the `default` option of
`thread_mattr_accessor` to be set in descendant classes
as well as in any new Thread that starts.

Previously, the `default` value provided was set only at the
moment of defining the attribute writer, which would cause
the attribute to be uninitialized in descendants and in other threads.

For instance:

  ```ruby
  class Processor
    thread_mattr_accessor :mode, default: :smart
  end

  class SubProcessor < Processor
  end

  SubProcessor.mode # => :smart (was `nil` prior to this commit)

  Thread.new do
    Processor.mode # => :smart (was `nil` prior to this commit)
  end.join
  ```

If a non-`nil` default has been specified, there is a small (~7%)
performance decrease when reading non-`nil` values, and a larger (~45%)
performance decrease when reading `nil` values.

Benchmark script:

  ```ruby
  # frozen_string_literal: true
  require "benchmark/ips"
  require "active_support"
  require "active_support/core_ext/module/attribute_accessors_per_thread"

  class MyClass
    thread_mattr_accessor :default_value, default: "default"
    thread_mattr_accessor :string_value, default: "default"
    thread_mattr_accessor :nil_value, default: "default"
  end

  MyClass.string_value = "string"
  MyClass.nil_value = nil

  Benchmark.ips do |x|
    x.report("default_value") { MyClass.default_value }
    x.report("string_value") { MyClass.string_value }
    x.report("nil_value") { MyClass.nil_value }
  end
  ```

Before this commit:

  ```
         default_value      2.075M (± 0.7%) i/s -     10.396M in   5.010585s
          string_value      2.103M (± 0.7%) i/s -     10.672M in   5.074624s
             nil_value      1.777M (± 0.9%) i/s -      8.924M in   5.023058s
  ```

After this commit:

  ```
         default_value      2.008M (± 0.7%) i/s -     10.187M in   5.072990s
          string_value      1.967M (± 0.7%) i/s -      9.891M in   5.028570s
             nil_value      1.144M (± 0.5%) i/s -      5.770M in   5.041630s
  ```

If no default or a `nil` default is specified, there is no performance
impact.

Fixes rails#43312.

Co-authored-by: Jonathan Hefner <jonathan@hefner.pro>

activesupport/CHANGELOG.md

@@ -1,3 +1,14 @@
+*   `default` option of `thread_mattr_accessor` now applies through inheritance and
+    also across new threads.
+
+    Previously, the `default` value provided was set only at the moment of defining
+    the attribute writer, which would cause the attribute to be uninitialized in
+    descendants and in other threads.
+
+    Fixes #43312.
+
+    *Thierry Deo*
+
 *   Redis cache store is now compatible with redis-rb 5.0.
 
     *Jean Boussier*

activesupport/lib/active_support/core_ext/module/attribute_accessors_per_thread.rb

@@ -44,12 +44,29 @@ def thread_mattr_reader(*syms, instance_reader: true, instance_accessor: true, d
 
       # The following generated method concatenates `name` because we want it
       # to work with inheritance via polymorphism.
-      class_eval(<<-EOS, __FILE__, __LINE__ + 1)
-        def self.#{sym}
-          @__thread_mattr_#{sym} ||= "attr_\#{name}_#{sym}"
-          ::ActiveSupport::IsolatedExecutionState[@__thread_mattr_#{sym}]
-        end
-      EOS
+      if default.nil?
+        class_eval(<<-EOS, __FILE__, __LINE__ + 1)
+          def self.#{sym}
+            @__thread_mattr_#{sym} ||= "attr_\#{name}_#{sym}"
+            ::ActiveSupport::IsolatedExecutionState[@__thread_mattr_#{sym}]
+          end
+        EOS
+      else
+        singleton_class.define_method("#{sym}_default_value") { default }
+
+        class_eval(<<-EOS, __FILE__, __LINE__ + 1)
+          def self.#{sym}
+            @__thread_mattr_#{sym} ||= "attr_\#{name}_#{sym}"
+            value = ::ActiveSupport::IsolatedExecutionState[@__thread_mattr_#{sym}]
+
+            if value.nil? && !::ActiveSupport::IsolatedExecutionState.key?(@__thread_mattr_#{sym})
+              ::ActiveSupport::IsolatedExecutionState[@__thread_mattr_#{sym}] = #{sym}_default_value
+            else
+              value
+            end
+          end
+        EOS
+      end
 
       if instance_reader && instance_accessor
         class_eval(<<-EOS, __FILE__, __LINE__ + 1)
@@ -58,8 +75,6 @@ def #{sym}
           end
         EOS
       end
-
-      ::ActiveSupport::IsolatedExecutionState["attr_#{name}_#{sym}"] = default unless default.nil?
     end
   end
   alias :thread_cattr_reader :thread_mattr_reader
@@ -82,7 +97,7 @@ def #{sym}
   #   end
   #
   #   Current.new.user = "DHH" # => NoMethodError
-  def thread_mattr_writer(*syms, instance_writer: true, instance_accessor: true, default: nil) # :nodoc:
+  def thread_mattr_writer(*syms, instance_writer: true, instance_accessor: true) # :nodoc:
     syms.each do |sym|
       raise NameError.new("invalid attribute name: #{sym}") unless /^[_A-Za-z]\w*$/.match?(sym)
 
@@ -102,8 +117,6 @@ def #{sym}=(obj)
           end
         EOS
       end
-
-      public_send("#{sym}=", default) unless default.nil?
     end
   end
   alias :thread_cattr_writer :thread_mattr_writer

activesupport/test/core_ext/module/attribute_accessor_per_thread_test.rb

@@ -4,19 +4,20 @@
 require "active_support/core_ext/module/attribute_accessors_per_thread"
 
 class ModuleAttributeAccessorPerThreadTest < ActiveSupport::TestCase
-  class MyClass
-    thread_mattr_accessor :foo
-    thread_mattr_accessor :bar,  instance_writer: false
-    thread_mattr_reader   :shaq, instance_reader: false
-    thread_mattr_accessor :camp, instance_accessor: false
-  end
+  setup do
+    @class = Class.new do
+      def self.name; "MyClass#{object_id}"; end
 
-  class SubMyClass < MyClass
-  end
+      thread_mattr_accessor :foo
+      thread_mattr_accessor :bar,  instance_writer: false
+      thread_mattr_reader   :shaq, instance_reader: false
+      thread_mattr_accessor :camp, instance_accessor: false
+    end
+
+    @subclass = Class.new(@class) do
+      def self.name; "Sub#{super}"; end
+    end
 
-  setup do
-    @class = MyClass
-    @subclass = SubMyClass
     @object = @class.new
   end
 
@@ -41,14 +42,35 @@ def test_is_not_shared_between_fibers_if_isolation_level_is_fiber
     ActiveSupport::IsolatedExecutionState.isolation_level = previous_level
   end
 
-  def test_can_initialize_with_default_value
-    Thread.new do
-      @class.thread_mattr_accessor :baz, default: "default_value"
+  def test_default_value
+    @class.thread_mattr_accessor :baz, default: "default_value"
 
+    assert_equal "default_value", @class.baz
+  end
+
+  def test_default_value_is_accessible_from_subclasses
+    @class.thread_mattr_accessor :baz, default: "default_value"
+
+    assert_equal "default_value", @subclass.baz
+  end
+
+  def test_default_value_is_accessible_from_other_threads
+    @class.thread_mattr_accessor :baz, default: "default_value"
+
+    Thread.new do
       assert_equal "default_value", @class.baz
     end.join
+  end
+
+  def test_default_value_is_the_same_object
+    default = Object.new
+    @class.thread_mattr_accessor :baz, default: default
 
-    assert_nil @class.baz
+    assert_same default, @class.baz
+
+    Thread.new do
+      assert_same default, @class.baz
+    end.join
   end
 
   def test_should_use_mattr_default
@@ -160,4 +182,28 @@ def test_should_not_affect_superclass_if_subclass_set_value
     assert_equal "super", @class.foo
     assert_equal "sub", @subclass.foo
   end
+
+  def test_superclass_keeps_default_value_when_value_set_on_subclass
+    @class.thread_mattr_accessor :baz, default: "default_value"
+    @subclass.baz = "sub"
+
+    assert_equal "default_value", @class.baz
+    assert_equal "sub", @subclass.baz
+  end
+
+  def test_subclass_keeps_default_value_when_value_set_on_superclass
+    @class.thread_mattr_accessor :baz, default: "default_value"
+    @class.baz = "super"
+
+    assert_equal "super", @class.baz
+    assert_equal "default_value", @subclass.baz
+  end
+
+  def test_subclass_can_override_default_value_without_affecting_superclass
+    @class.thread_mattr_accessor :baz, default: "super"
+    @subclass.thread_mattr_accessor :baz, default: "sub"
+
+    assert_equal "super", @class.baz
+    assert_equal "sub", @subclass.baz
+  end
 end