remove_instance_variable: Handle running out of shapes

`remove_shape_recursive` wasn't considering that if we run out of
shapes, it might have to transition to SHAPE_TOO_COMPLEX.

When this happens, we now return with an error and the caller
initiates the evacuation.

shape.c

@@ -585,6 +585,10 @@ remove_shape_recursive(VALUE obj, ID id, rb_shape_t * shape, VALUE * removed)
             if (new_parent) {
                 bool dont_care;
                 rb_shape_t * new_child = get_next_shape_internal(new_parent, shape->edge_name, shape->type, &dont_care, true);
+                if (UNLIKELY(new_parent->type == SHAPE_OBJ_TOO_COMPLEX)) {
+                    return new_parent;
+                }
+
                 new_child->capacity = shape->capacity;
                 if (new_child->type == SHAPE_IVAR) {
                     move_iv(obj, id, shape->next_iv_index - 1, new_child->next_iv_index - 1);
@@ -601,13 +605,22 @@ remove_shape_recursive(VALUE obj, ID id, rb_shape_t * shape, VALUE * removed)
     }
 }
 
-void
+bool
 rb_shape_transition_shape_remove_ivar(VALUE obj, ID id, rb_shape_t *shape, VALUE * removed)
 {
+    if (UNLIKELY(shape->type == SHAPE_OBJ_TOO_COMPLEX)) {
+        return false;
+    }
+
     rb_shape_t * new_shape = remove_shape_recursive(obj, id, shape, removed);
     if (new_shape) {
+        if (UNLIKELY(new_shape->type == SHAPE_OBJ_TOO_COMPLEX)) {
+            return false;
+        }
+
         rb_shape_set_shape(obj, new_shape);
     }
+    return true;
 }
 
 rb_shape_t *

shape.h

@@ -162,7 +162,7 @@ void rb_shape_set_shape(VALUE obj, rb_shape_t* shape);
 rb_shape_t* rb_shape_get_shape(VALUE obj);
 int rb_shape_frozen_shape_p(rb_shape_t* shape);
 rb_shape_t* rb_shape_transition_shape_frozen(VALUE obj);
-void rb_shape_transition_shape_remove_ivar(VALUE obj, ID id, rb_shape_t *shape, VALUE * removed);
+bool rb_shape_transition_shape_remove_ivar(VALUE obj, ID id, rb_shape_t *shape, VALUE * removed);
 rb_shape_t * rb_shape_transition_shape_capa(rb_shape_t * shape);
 rb_shape_t* rb_shape_get_next(rb_shape_t* shape, VALUE obj, ID id);
 

test/ruby/test_shapes.rb

@@ -302,6 +302,38 @@ class TestShapes < Test::Unit::TestCase
     end;
   end
 
+  def test_run_out_of_shape_remove_instance_variable
+    assert_separately([], "#{<<~"begin;"}\n#{<<~'end;'}")
+    begin;
+      class A
+        attr_reader :a, :b, :c, :d
+        def initialize
+          @a = @b = @c = @d = 1
+        end
+      end
+
+      a = A.new
+
+      o = Object.new
+      i = 0
+      while RubyVM::Shape.shapes_available > 0
+        o.instance_variable_set(:"@i#{i}", 1)
+        i += 1
+      end
+
+      a.remove_instance_variable(:@b)
+      assert_nil a.b
+
+      a.remove_instance_variable(:@a)
+      assert_nil a.a
+
+      a.remove_instance_variable(:@c)
+      assert_nil a.c
+
+      assert_equal 1, a.d
+    end;
+  end
+
   def test_run_out_of_shape_rb_obj_copy_ivar
     assert_separately([], "#{<<~"begin;"}\n#{<<~'end;'}")
     begin;
@@ -501,6 +533,26 @@ class TestShapes < Test::Unit::TestCase
     assert_nil tc.a3
   end
 
+  def test_remove_instance_variable
+    ivars_count = 5
+    object = Object.new
+    ivars_count.times do |i|
+      object.instance_variable_set("@ivar_#{i}", i)
+    end
+
+    ivars = ivars_count.times.map do |i|
+      object.instance_variable_get("@ivar_#{i}")
+    end
+    assert_equal [0, 1, 2, 3, 4], ivars
+
+    object.remove_instance_variable(:@ivar_2)
+
+    ivars = ivars_count.times.map do |i|
+      object.instance_variable_get("@ivar_#{i}")
+    end
+    assert_equal [0, 1, nil, 3, 4], ivars
+  end
+
   def test_freeze_after_complex
     ensure_complex
 

variable.c

@@ -2198,28 +2198,32 @@ rb_obj_remove_instance_variable(VALUE obj, VALUE name)
       case T_CLASS:
       case T_MODULE:
         IVAR_ACCESSOR_SHOULD_BE_MAIN_RACTOR(id);
-        if (rb_shape_obj_too_complex(obj)) {
+        if (!rb_shape_transition_shape_remove_ivar(obj, id, shape, &val)) {
+            if (!rb_shape_obj_too_complex(obj)) {
+                rb_evict_ivars_to_hash(obj, shape);
+            }
             if (!st_delete(RCLASS_IV_HASH(obj), (st_data_t *)&id, (st_data_t *)&val)) {
                 val = Qundef;
             }
         }
-        else {
-            rb_shape_transition_shape_remove_ivar(obj, id, shape, &val);
-        }
         break;
       case T_OBJECT: {
-        if (rb_shape_obj_too_complex(obj)) {
+        if (!rb_shape_transition_shape_remove_ivar(obj, id, shape, &val)) {
+            if (!rb_shape_obj_too_complex(obj)) {
+                rb_evict_ivars_to_hash(obj, shape);
+            }
             if (rb_st_lookup(ROBJECT_IV_HASH(obj), (st_data_t)id, (st_data_t *)&val)) {
                 rb_st_delete(ROBJECT_IV_HASH(obj), (st_data_t *)&id, 0);
             }
         }
-        else {
-            rb_shape_transition_shape_remove_ivar(obj, id, shape, &val);
-        }
         break;
       }
       default: {
-        if (rb_shape_obj_too_complex(obj)) {
+        if (!rb_shape_transition_shape_remove_ivar(obj, id, shape, &val)) {
+            if (!rb_shape_obj_too_complex(obj)) {
+                rb_evict_ivars_to_hash(obj, shape);
+            }
+
             struct gen_ivtbl *ivtbl;
             if (rb_gen_ivtbl_get(obj, 0, &ivtbl)) {
                 if (!st_delete(ivtbl->as.complex.table, (st_data_t *)&id, (st_data_t *)&val)) {
@@ -2227,9 +2231,6 @@ rb_obj_remove_instance_variable(VALUE obj, VALUE name)
                 }
             }
         }
-        else {
-            rb_shape_transition_shape_remove_ivar(obj, id, shape, &val);
-        }
         break;
       }
     }