AtomicMarkableReference: compare object identities if not Numeric type

Objects should always be compared by reference so we can ensure that we are
referring to the *same* object, not simply one that maybe be considered
logically equivalent by overriding `==`.

This is, of course, unless it is a Numeric. When comparing something primitive
like a number, we expect that the equality operator makes mathematical sense.

Author: ianks

lib/concurrent/atomic/atomic_markable_reference.rb

@@ -30,19 +30,30 @@ def compare_and_set(expected_val, new_val, expected_mark, new_mark)
         current = @Reference.get
         curr_val, curr_mark = current
 
-        # Ensure that that the expected values match.
-        return false unless (expected_val == curr_val) &&
-                            (expected_mark == curr_mark)
+        # Ensure that that the expected marks match.
+        return false unless expected_mark == curr_mark
 
-        # In this case, it would be redundant to set the fields. Just
-        # shortcircuit without wasting CPU time on CAS.
-        return true if (new_val == curr_val) &&
-                       (new_mark == curr_mark)
+        if expected_val.is_a? Numeric
+          # If the object is a numeric, we need to ensure we are comparing
+          # the numerical values
+          return false unless expected_val == curr_val
+        else
+          # Otherwise, we need to ensure we are comparing the object identity.
+          # Theoretically, this could be incorrect if a user monkey-patched
+          # `Object#equal?`, but they should know that they are playing with
+          # fire at that point.
+          return false unless expected_val.equal? curr_val
+        end
 
         prospect = ImmutableArray[new_val, new_mark]
 
-        @Reference.compare_and_set current, prospect
+        # If we guarantee internally that `current` will never be a Numeric, we
+        # can skip a type check in `compare_and_set` and directly call
+        # `_compare_and_set`. This is possible since we always internally wrap
+        # the users `value` and `mark` in an ImmutableArray.
+        @Reference._compare_and_set current, prospect
       end
+      alias_method :compare_and_swap, :compare_and_set
 
       # @!macro [attach] atomic_markable_reference_method_get
       #

spec/concurrent/atomic/atomic_markable_reference_spec.rb

@@ -83,4 +83,22 @@
     expect(subject.compare_and_set(Complex(1, 2), Complex(1, 3), false, true))
       .to be_truthy, "CAS failed for (#{Complex(1, 2)}, false) => (0, false)"
   end
+
+  describe '#compare_and_set' do
+    context 'objects have the same identity' do
+      it 'is successful' do
+        arr = [1, 2, 3]
+        subject.set(arr, true)
+        expect(subject.compare_and_set(arr, 1.2, true, false)).to be_truthy
+      end
+    end
+
+    context 'objects have the different identity' do
+      it 'is not successful' do
+        subject.set([1, 2, 3], true)
+        expect(subject.compare_and_set([1, 2, 3], 1.2, true, false))
+          .to be_falsey
+      end
+    end
+  end
 end