Fix thread-safety issues

Based on #2942 (comment)
Fixes #2897, Fixes #2942

Author: mshibuya

lib/rails_admin/config/configurable.rb

@@ -30,14 +30,16 @@ def with_recurring(option_name, value_proc, default_proc)
         # and allows configurations such as
         # label { "#{label}".upcase }
         # This will use the default definition when called recursively.
-        if instance_variable_get("@#{option_name}_recurring")
+        Thread.current[:rails_admin_recurring] ||= {}
+        Thread.current[:rails_admin_recurring][self] ||= {}
+        if Thread.current[:rails_admin_recurring][self][option_name]
           instance_eval(&default_proc)
         else
-          instance_variable_set("@#{option_name}_recurring", true)
+          Thread.current[:rails_admin_recurring][self][option_name] = true
           instance_eval(&value_proc)
         end
       ensure
-        instance_variable_set("@#{option_name}_recurring", false)
+        Thread.current[:rails_admin_recurring].delete(self)
       end
 
       module ClassMethods

lib/rails_admin/config/fields/base.rb

@@ -20,7 +20,7 @@ class Base # rubocop:disable ClassLength
 
         NAMED_INSTANCE_VARIABLES = [
           :@parent, :@root, :@section, :@children_fields_registered,
-          :@associated_model_config, :@group, :@bindings
+          :@associated_model_config, :@group
         ].freeze
 
         def initialize(parent, name, properties)

lib/rails_admin/config/proxyable.rb

@@ -2,7 +2,19 @@
 module RailsAdmin
   module Config
     module Proxyable
-      attr_accessor :bindings
+      def bindings
+        Thread.current[:rails_admin_bindings] ||= {}
+        Thread.current[:rails_admin_bindings][self]
+      end
+
+      def bindings=(new_bindings)
+        Thread.current[:rails_admin_bindings] ||= {}
+        if new_bindings.nil?
+          Thread.current[:rails_admin_bindings].delete(self)
+        else
+          Thread.current[:rails_admin_bindings][self] = new_bindings
+        end
+      end
 
       def with(bindings = {})
         RailsAdmin::Config::Proxyable::Proxy.new(self, bindings)

lib/rails_admin/config/proxyable/proxy.rb

@@ -2,8 +2,6 @@ module RailsAdmin
   module Config
     module Proxyable
       class Proxy < BasicObject
-        attr_reader :bindings
-
         def initialize(object, bindings = {})
           @object = object
           @bindings = bindings
@@ -21,12 +19,12 @@ def bind(key, value = nil)
 
         def method_missing(name, *args, &block)
           if @object.respond_to?(name)
-            reset = @object.instance_variable_get('@bindings')
+            reset = @object.bindings
             begin
-              @object.instance_variable_set('@bindings', @bindings)
+              @object.bindings = @bindings
               response = @object.__send__(name, *args, &block)
             ensure
-              @object.instance_variable_set('@bindings', reset)
+              @object.bindings = reset
             end
             response
           else

spec/rails_admin/config/configurable_spec.rb

@@ -0,0 +1,43 @@
+require 'spec_helper'
+
+describe RailsAdmin::Config::Configurable do
+  class ConfigurableTest
+    include RailsAdmin::Config::Configurable
+
+    register_instance_option :foo do
+      'default'
+    end
+  end
+
+  subject { ConfigurableTest.new }
+
+  describe 'recursion tracking' do
+    it 'works and use default value' do
+      subject.instance_eval do
+        foo { foo }
+      end
+      expect(subject.foo).to eq 'default'
+    end
+
+    describe 'with parallel execution' do
+      before do
+        subject.instance_eval do
+          foo do
+            sleep 0.15
+            'value'
+          end
+        end
+      end
+
+      it 'ensures thread-safety' do
+        threads = Array.new(2) do |i|
+          Thread.new do
+            sleep i * 0.1
+            expect(subject.foo).to eq 'value'
+          end
+        end
+        threads.each(&:join)
+      end
+    end
+  end
+end

spec/rails_admin/config/proxyable/proxy_spec.rb

@@ -0,0 +1,64 @@
+require 'spec_helper'
+
+describe RailsAdmin::Config::Proxyable do
+  class ProxyableTest
+    include RailsAdmin::Config::Proxyable
+
+    def boo
+      sleep 0.15
+      bindings[:foo]
+    end
+
+    def qux
+      'foobar'
+    end
+  end
+
+  let!(:proxyable_test) { ProxyableTest.new }
+  subject do
+    proxyable_test.bindings = {foo: 'bar'}
+    proxyable_test
+  end
+
+  it 'proxies method calls to @object' do
+    expect(subject.bindings).to eq foo: 'bar'
+  end
+
+  it 'preserves initially set @bindings' do
+    expect(subject.with(foo: 'baz').tap(&:qux).bindings).to eq foo: 'bar'
+  end
+
+  context 'when a method is defined in Kernel' do
+    before do
+      Kernel.module_eval do
+        def qux
+          'quux'
+        end
+      end
+    end
+
+    after do
+      Kernel.module_eval do
+        undef qux
+      end
+    end
+
+    it 'proxies calls for the method to @object' do
+      expect(subject.qux).to eq 'foobar'
+    end
+  end
+
+  describe 'with parallel execution' do
+    it 'ensures thread-safety' do
+      threads = Array.new(2) do |i|
+        Thread.new do
+          value = %w(a b)[i]
+          proxy = proxyable_test.with foo: value
+          sleep i * 0.1
+          expect(proxy.boo).to eq value
+        end
+      end
+      threads.each(&:join)
+    end
+  end
+end