Merge pull request rails#48053 from fatkodima/fix-find_or_create_by-transactions

Fix `find_or_create_by`/`find_or_create_by!` when used within concurrent transactions

Author: byroot

activerecord/lib/active_record/relation.rb

@@ -215,7 +215,11 @@ def find_or_create_by!(attributes, &block)
     def create_or_find_by(attributes, &block)
       transaction(requires_new: true) { create(attributes, &block) }
     rescue ActiveRecord::RecordNotUnique
-      find_by!(attributes)
+      if connection.transaction_open?
+        where(attributes).lock.find_by!(attributes)
+      else
+        find_by!(attributes)
+      end
     end
 
     # Like #create_or_find_by, but calls
@@ -224,7 +228,11 @@ def create_or_find_by(attributes, &block)
     def create_or_find_by!(attributes, &block)
       transaction(requires_new: true) { create!(attributes, &block) }
     rescue ActiveRecord::RecordNotUnique
-      find_by!(attributes)
+      if connection.transaction_open?
+        where(attributes).lock.find_by!(attributes)
+      else
+        find_by!(attributes)
+      end
     end
 
     # Like #find_or_create_by, but calls {new}[rdoc-ref:Core#new]

activerecord/test/cases/relations_test.rb

@@ -2452,3 +2452,53 @@ def custom_post_relation(alias_name = "omg_posts")
       )
     end
 end
+
+class CreateOrFindByWithinTransactions < ActiveRecord::TestCase
+  unless current_adapter?(:SQLite3Adapter)
+    self.use_transactional_tests = false
+
+    def teardown
+      Subscriber.delete_all
+    end
+
+    def test_multiple_find_or_create_by_within_transactions
+      duel { Subscriber.find_or_create_by(nick: "bob") }
+    end
+
+    def test_multiple_find_or_create_by_bang_within_transactions
+      duel { Subscriber.find_or_create_by!(nick: "bob") }
+    end
+
+    private
+      def duel
+        assert_nil Subscriber.find_by(nick: "bob")
+
+        a_wakeup = Concurrent::Event.new
+        b_wakeup = Concurrent::Event.new
+
+        a = Thread.new do
+          Subscriber.transaction do
+            a_wakeup.wait
+            yield
+            b_wakeup.set
+          end
+        end
+
+        b = Thread.new do
+          Subscriber.transaction do
+            # Read the record prematurely for MySQL REPEATABLE READ to kick in
+            Subscriber.find_by(nick: "bob")
+
+            a_wakeup.set
+            b_wakeup.wait
+            yield
+          end
+        end
+
+        a.join
+        b.join
+
+        assert_equal 1, Subscriber.where(nick: "bob").count
+      end
+  end
+end