Don't pin named structs defined in Ruby

[Bug #20311]

`rb_define_class_under` assumes it's called from C and that the
reference might be held in a C global variable, so it adds the
class to the VM root.

In the case of `Struct.new('Name')` it's wasteful and make
the struct immortal.

Author: byroot

class.c

@@ -1007,7 +1007,7 @@ rb_define_class_under(VALUE outer, const char *name, VALUE super)
 }
 
 VALUE
-rb_define_class_id_under(VALUE outer, ID id, VALUE super)
+rb_define_class_id_under_no_pin(VALUE outer, ID id, VALUE super)
 {
     VALUE klass;
 
@@ -1024,8 +1024,6 @@ rb_define_class_id_under(VALUE outer, ID id, VALUE super)
                      " (%"PRIsVALUE" is given but was %"PRIsVALUE")",
                      outer, rb_id2str(id), RCLASS_SUPER(klass), super);
         }
-        /* Class may have been defined in Ruby and not pin-rooted */
-        rb_vm_add_root_module(klass);
 
         return klass;
     }
@@ -1037,11 +1035,18 @@ rb_define_class_id_under(VALUE outer, ID id, VALUE super)
     rb_set_class_path_string(klass, outer, rb_id2str(id));
     rb_const_set(outer, id, klass);
     rb_class_inherited(super, klass);
-    rb_vm_add_root_module(klass);
 
     return klass;
 }
 
+VALUE
+rb_define_class_id_under(VALUE outer, ID id, VALUE super)
+{
+    VALUE klass = rb_define_class_id_under_no_pin(outer, id, super);
+    rb_vm_add_root_module(klass);
+    return klass;
+}
+
 VALUE
 rb_module_s_alloc(VALUE klass)
 {

internal/class.h

@@ -175,6 +175,7 @@ void rb_class_foreach_subclass(VALUE klass, void (*f)(VALUE, VALUE), VALUE);
 void rb_class_detach_subclasses(VALUE);
 void rb_class_detach_module_subclasses(VALUE);
 void rb_class_remove_from_module_subclasses(VALUE);
+VALUE rb_define_class_id_under_no_pin(VALUE outer, ID id, VALUE super);
 VALUE rb_obj_methods(int argc, const VALUE *argv, VALUE obj);
 VALUE rb_obj_protected_methods(int argc, const VALUE *argv, VALUE obj);
 VALUE rb_obj_private_methods(int argc, const VALUE *argv, VALUE obj);

struct.c

@@ -273,7 +273,7 @@ new_struct(VALUE name, VALUE super)
         rb_warn("redefining constant %"PRIsVALUE"::%"PRIsVALUE, super, name);
         rb_mod_remove_const(super, ID2SYM(id));
     }
-    return rb_define_class_id_under(super, id, super);
+    return rb_define_class_id_under_no_pin(super, id, super);
 }
 
 NORETURN(static void invalid_struct_pos(VALUE s, VALUE idx));
@@ -491,8 +491,13 @@ rb_struct_define(const char *name, ...)
     ary = struct_make_members_list(ar);
     va_end(ar);
 
-    if (!name) st = anonymous_struct(rb_cStruct);
-    else st = new_struct(rb_str_new2(name), rb_cStruct);
+    if (!name) {
+        st = anonymous_struct(rb_cStruct);
+    }
+    else {
+        st = new_struct(rb_str_new2(name), rb_cStruct);
+        rb_vm_add_root_module(st);
+    }
     return setup_struct(st, ary);
 }
 
@@ -506,7 +511,7 @@ rb_struct_define_under(VALUE outer, const char *name, ...)
     ary = struct_make_members_list(ar);
     va_end(ar);
 
-    return setup_struct(rb_define_class_under(outer, name, rb_cStruct), ary);
+    return setup_struct(rb_define_class_id_under(outer, rb_intern(name), rb_cStruct), ary);
 }
 
 /*
@@ -1699,7 +1704,9 @@ rb_data_define(VALUE super, ...)
     ary = struct_make_members_list(ar);
     va_end(ar);
     if (!super) super = rb_cData;
-    return setup_data(anonymous_struct(super), ary);
+    VALUE klass = setup_data(anonymous_struct(super), ary);
+    rb_vm_add_root_module(klass);
+    return klass;
 }
 
 /*

test/ruby/test_struct.rb

@@ -534,6 +534,20 @@ def test_parameters
     assert_equal [[:req, :_]], klass.instance_method(:c=).parameters
   end
 
+  def test_named_structs_are_not_rooted
+    # [Bug #20311]
+    assert_no_memory_leak([], <<~PREP, <<~CODE, rss: true)
+      code = proc do
+        Struct.new("A")
+        Struct.send(:remove_const, :A)
+      end
+
+      1_000.times(&code)
+    PREP
+      300_000.times(&code)
+    CODE
+  end
+
   class TopStruct < Test::Unit::TestCase
     include TestStruct