Make inline cache reads / writes atomic with object shapes

Prior to this commit, we were reading and writing ivar index and
shape ID in inline caches in two separate instructions when
getting and setting ivars. This meant there was a race condition
with ractors and these caches where one ractor could change
a value in the cache while another was still reading from it.

This commit instead reads and writes shape ID and ivar index to
inline caches atomically so there is no longer a race condition.

Co-Authored-By: Aaron Patterson <tenderlove@ruby-lang.org>
Co-Authored-By: John Hawthorn <john@hawthorn.email>

1 files changed, 33 insertions, 24 deletions

diff --git a/test/ruby/test_shapes.rb b/test/ruby/test_shapes.rb
index 807d485354..7142c30cd5 100644
--- a/test/ruby/test_shapes.rb
+++ b/test/ruby/test_shapes.rb
@@ -23,7 +23,7 @@ class TestShapes < Test::Unit::TestCase
     end
   end
 
-  # RubyVM.debug_shape returns new instances of shape objects for
+  # RubyVM::Shape.of returns new instances of shape objects for
   # each call. This helper method allows us to define equality for
   # shapes
   def assert_shape_equal(shape1, shape2)
@@ -39,63 +39,63 @@ class TestShapes < Test::Unit::TestCase
 
   def test_iv_index
     example = RemoveAndAdd.new
-    shape = RubyVM.debug_shape(example)
+    shape = RubyVM::Shape.of(example)
     assert_equal 0, shape.iv_count
 
     example.add_foo # makes a transition
-    new_shape = RubyVM.debug_shape(example)
+    new_shape = RubyVM::Shape.of(example)
     assert_equal([:@foo], example.instance_variables)
     assert_equal(shape.id, new_shape.parent.id)
     assert_equal(1, new_shape.iv_count)
 
     example.remove # makes a transition
-    remove_shape = RubyVM.debug_shape(example)
+    remove_shape = RubyVM::Shape.of(example)
     assert_equal([], example.instance_variables)
     assert_equal(new_shape.id, remove_shape.parent.id)
     assert_equal(1, remove_shape.iv_count)
 
     example.add_bar # makes a transition
-    bar_shape = RubyVM.debug_shape(example)
+    bar_shape = RubyVM::Shape.of(example)
     assert_equal([:@bar], example.instance_variables)
     assert_equal(remove_shape.id, bar_shape.parent.id)
     assert_equal(2, bar_shape.iv_count)
   end
 
   def test_new_obj_has_root_shape
-    assert_shape_equal(RubyVM.debug_root_shape, RubyVM.debug_shape(Object.new))
+    assert_shape_equal(RubyVM::Shape.root_shape, RubyVM::Shape.of(Object.new))
   end
 
   def test_frozen_new_obj_has_frozen_root_shape
     assert_shape_equal(
-      RubyVM.debug_frozen_root_shape,
-      RubyVM.debug_shape(Object.new.freeze)
+      RubyVM::Shape.frozen_root_shape,
+      RubyVM::Shape.of(Object.new.freeze)
     )
   end
 
   def test_str_has_root_shape
-    assert_shape_equal(RubyVM.debug_root_shape, RubyVM.debug_shape(""))
+    assert_shape_equal(RubyVM::Shape.root_shape, RubyVM::Shape.of(""))
   end
 
   def test_array_has_root_shape
-    assert_shape_equal(RubyVM.debug_root_shape, RubyVM.debug_shape([]))
+    assert_shape_equal(RubyVM::Shape.root_shape, RubyVM::Shape.of([]))
   end
 
   def test_hash_has_root_shape
-    assert_shape_equal(RubyVM.debug_root_shape, RubyVM.debug_shape({}))
+    assert_shape_equal(RubyVM::Shape.root_shape, RubyVM::Shape.of({}))
   end
 
   def test_true_has_frozen_root_shape
-    assert_shape_equal(RubyVM.debug_frozen_root_shape, RubyVM.debug_shape(true))
+    assert_shape_equal(RubyVM::Shape.frozen_root_shape, RubyVM::Shape.of(true))
   end
 
   def test_nil_has_frozen_root_shape
-    assert_shape_equal(RubyVM.debug_frozen_root_shape, RubyVM.debug_shape(nil))
+    assert_shape_equal(RubyVM::Shape.frozen_root_shape, RubyVM::Shape.of(nil))
   end
 
   def test_basic_shape_transition
     obj = Example.new
-    refute_equal(RubyVM.debug_root_shape, RubyVM.debug_shape(obj))
-    assert_shape_equal(RubyVM.debug_root_shape.edges[:@a], RubyVM.debug_shape(obj))
+    refute_equal(RubyVM::Shape.root_shape, RubyVM::Shape.of(obj))
+    assert_shape_equal(RubyVM::Shape.root_shape.edges[:@a], RubyVM::Shape.of(obj))
     assert_equal(obj.instance_variable_get(:@a), 1)
   end
 
@@ -103,13 +103,13 @@ class TestShapes < Test::Unit::TestCase
     obj = Example.new
     obj2 = ""
     obj2.instance_variable_set(:@a, 1)
-    assert_shape_equal(RubyVM.debug_shape(obj), RubyVM.debug_shape(obj2))
+    assert_shape_equal(RubyVM::Shape.of(obj), RubyVM::Shape.of(obj2))
   end
 
   def test_duplicating_objects
     obj = Example.new
     obj2 = obj.dup
-    assert_shape_equal(RubyVM.debug_shape(obj), RubyVM.debug_shape(obj2))
+    assert_shape_equal(RubyVM::Shape.of(obj), RubyVM::Shape.of(obj2))
   end
 
   def test_freezing_and_duplicating_object
@@ -118,14 +118,14 @@ class TestShapes < Test::Unit::TestCase
     refute_predicate(obj2, :frozen?)
     # dup'd objects shouldn't be frozen, and the shape should be the
     # parent shape of the copied object
-    assert_equal(RubyVM.debug_shape(obj).parent.id, RubyVM.debug_shape(obj2).id)
+    assert_equal(RubyVM::Shape.of(obj).parent.id, RubyVM::Shape.of(obj2).id)
   end
 
   def test_freezing_and_duplicating_object_with_ivars
     obj = Example.new.freeze
     obj2 = obj.dup
     refute_predicate(obj2, :frozen?)
-    refute_shape_equal(RubyVM.debug_shape(obj), RubyVM.debug_shape(obj2))
+    refute_shape_equal(RubyVM::Shape.of(obj), RubyVM::Shape.of(obj2))
     assert_equal(obj2.instance_variable_get(:@a), 1)
   end
 
@@ -135,7 +135,7 @@ class TestShapes < Test::Unit::TestCase
     str.freeze
     str2 = str.dup
     refute_predicate(str2, :frozen?)
-    refute_equal(RubyVM.debug_shape(str).id, RubyVM.debug_shape(str2).id)
+    refute_equal(RubyVM::Shape.of(str).id, RubyVM::Shape.of(str2).id)
     assert_equal(str2.instance_variable_get(:@a), 1)
   end
 
@@ -143,14 +143,14 @@ class TestShapes < Test::Unit::TestCase
     obj = Object.new.freeze
     obj2 = obj.clone(freeze: true)
     assert_predicate(obj2, :frozen?)
-    assert_shape_equal(RubyVM.debug_shape(obj), RubyVM.debug_shape(obj2))
+    assert_shape_equal(RubyVM::Shape.of(obj), RubyVM::Shape.of(obj2))
   end
 
   def test_freezing_and_cloning_object_with_ivars
     obj = Example.new.freeze
     obj2 = obj.clone(freeze: true)
     assert_predicate(obj2, :frozen?)
-    assert_shape_equal(RubyVM.debug_shape(obj), RubyVM.debug_shape(obj2))
+    assert_shape_equal(RubyVM::Shape.of(obj), RubyVM::Shape.of(obj2))
     assert_equal(obj2.instance_variable_get(:@a), 1)
   end
 
@@ -158,7 +158,7 @@ class TestShapes < Test::Unit::TestCase
     str = "str".freeze
     str2 = str.clone(freeze: true)
     assert_predicate(str2, :frozen?)
-    assert_shape_equal(RubyVM.debug_shape(str), RubyVM.debug_shape(str2))
+    assert_shape_equal(RubyVM::Shape.of(str), RubyVM::Shape.of(str2))
   end
 
   def test_freezing_and_cloning_string_with_ivars
@@ -167,7 +167,16 @@ class TestShapes < Test::Unit::TestCase
     str.freeze
     str2 = str.clone(freeze: true)
     assert_predicate(str2, :frozen?)
-    assert_shape_equal(RubyVM.debug_shape(str), RubyVM.debug_shape(str2))
+    assert_shape_equal(RubyVM::Shape.of(str), RubyVM::Shape.of(str2))
     assert_equal(str2.instance_variable_get(:@a), 1)
   end
+
+  def test_out_of_bounds_shape
+    assert_raise ArgumentError do
+      RubyVM::Shape.find_by_id(RubyVM::Shape.next_shape_id)
+    end
+    assert_raise ArgumentError do
+      RubyVM::Shape.find_by_id(-1)
+    end
+  end
 end