Merge pull request #472 from film42/master

Make Visitor visit thread safe by holding dispatch method reference

1 files changed, 49 insertions, 0 deletions

diff --git a/test/visitors/test_dispatch_contamination.rb b/test/visitors/test_dispatch_contamination.rb
index 6422a6dff3..71792594d6 100644
--- a/test/visitors/test_dispatch_contamination.rb
+++ b/test/visitors/test_dispatch_contamination.rb
@@ -1,8 +1,42 @@
 # frozen_string_literal: true
 require 'helper'
+require 'concurrent'
 
 module Arel
   module Visitors
+    class DummyVisitor < Visitor
+      def initialize
+        super
+        @barrier = Concurrent::CyclicBarrier.new(2)
+      end
+
+      def visit_Arel_Visitors_DummySuperNode node
+        42
+      end
+
+      # This is terrible, but it's the only way to reliably reproduce
+      # the possible race where two threads attempt to correct the
+      # dispatch hash at the same time.
+      def send *args
+        super
+      rescue
+        # Both threads try (and fail) to dispatch to the subclass's name
+        @barrier.wait
+        raise
+      ensure
+        # Then one thread successfully completes (updating the dispatch
+        # table in the process) before the other finishes raising its
+        # exception.
+        Thread.current[:delay].wait if Thread.current[:delay]
+      end
+    end
+
+    class DummySuperNode
+    end
+
+    class DummySubNode < DummySuperNode
+    end
+
     describe 'avoiding contamination between visitor dispatch tables' do
       before do
         @connection = Table.engine.connection
@@ -17,6 +51,21 @@ module Arel
 
         assert_equal "( TRUE UNION FALSE )", node.to_sql
       end
+
+      it 'is threadsafe when implementing superclass fallback' do
+        visitor = DummyVisitor.new
+        main_thread_finished = Concurrent::Event.new
+
+        racing_thread = Thread.new do
+          Thread.current[:delay] = main_thread_finished
+          visitor.accept DummySubNode.new
+        end
+
+        assert_equal 42, visitor.accept(DummySubNode.new)
+        main_thread_finished.set
+
+        assert_equal 42, racing_thread.value
+      end
     end
   end
 end