merge revision(s) e626da82ea, f3af5ae7e6: [Backport #20311]

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.

	Make Struct memory leak test faster

	[Bug #20311]

	It times out on some platform, so we can reduce iterations.
	On my machine it completes in 250ms and RSS grows 8X.
This commit is contained in:
nagachika 2024-07-07 14:47:04 +09:00
parent 2a4469ea59
commit bd5df1693c
5 changed files with 34 additions and 9 deletions

13
class.c
View file

@ -958,7 +958,7 @@ rb_define_class_under(VALUE outer, const char *name, VALUE super)
} }
VALUE 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; VALUE klass;
@ -975,8 +975,6 @@ rb_define_class_id_under(VALUE outer, ID id, VALUE super)
" (%"PRIsVALUE" is given but was %"PRIsVALUE")", " (%"PRIsVALUE" is given but was %"PRIsVALUE")",
outer, rb_id2str(id), RCLASS_SUPER(klass), super); 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; return klass;
} }
@ -988,11 +986,18 @@ rb_define_class_id_under(VALUE outer, ID id, VALUE super)
rb_set_class_path_string(klass, outer, rb_id2str(id)); rb_set_class_path_string(klass, outer, rb_id2str(id));
rb_const_set(outer, id, klass); rb_const_set(outer, id, klass);
rb_class_inherited(super, klass); rb_class_inherited(super, klass);
rb_vm_add_root_module(klass);
return 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 VALUE
rb_module_s_alloc(VALUE klass) rb_module_s_alloc(VALUE klass)
{ {

View file

@ -119,6 +119,7 @@ void rb_class_foreach_subclass(VALUE klass, void (*f)(VALUE, VALUE), VALUE);
void rb_class_detach_subclasses(VALUE); void rb_class_detach_subclasses(VALUE);
void rb_class_detach_module_subclasses(VALUE); void rb_class_detach_module_subclasses(VALUE);
void rb_class_remove_from_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_methods(int argc, const VALUE *argv, VALUE obj);
VALUE rb_obj_protected_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); VALUE rb_obj_private_methods(int argc, const VALUE *argv, VALUE obj);

View file

@ -274,7 +274,7 @@ new_struct(VALUE name, VALUE super)
rb_warn("redefining constant %"PRIsVALUE"::%"PRIsVALUE, super, name); rb_warn("redefining constant %"PRIsVALUE"::%"PRIsVALUE, super, name);
rb_mod_remove_const(super, ID2SYM(id)); 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)); NORETURN(static void invalid_struct_pos(VALUE s, VALUE idx));
@ -495,8 +495,13 @@ rb_struct_define(const char *name, ...)
ary = struct_make_members_list(ar); ary = struct_make_members_list(ar);
va_end(ar); va_end(ar);
if (!name) st = anonymous_struct(rb_cStruct); if (!name) {
else st = new_struct(rb_str_new2(name), rb_cStruct); 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); return setup_struct(st, ary);
} }
@ -510,7 +515,7 @@ rb_struct_define_under(VALUE outer, const char *name, ...)
ary = struct_make_members_list(ar); ary = struct_make_members_list(ar);
va_end(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);
} }
/* /*

View file

@ -526,6 +526,20 @@ module TestStruct
assert_equal [[:req, :_]], klass.instance_method(:c=).parameters assert_equal [[:req, :_]], klass.instance_method(:c=).parameters
end 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
50_000.times(&code)
CODE
end
class TopStruct < Test::Unit::TestCase class TopStruct < Test::Unit::TestCase
include TestStruct include TestStruct

View file

@ -11,7 +11,7 @@
# define RUBY_VERSION_MINOR RUBY_API_VERSION_MINOR # define RUBY_VERSION_MINOR RUBY_API_VERSION_MINOR
#define RUBY_VERSION_TEENY 4 #define RUBY_VERSION_TEENY 4
#define RUBY_RELEASE_DATE RUBY_RELEASE_YEAR_STR"-"RUBY_RELEASE_MONTH_STR"-"RUBY_RELEASE_DAY_STR #define RUBY_RELEASE_DATE RUBY_RELEASE_YEAR_STR"-"RUBY_RELEASE_MONTH_STR"-"RUBY_RELEASE_DAY_STR
#define RUBY_PATCHLEVEL 183 #define RUBY_PATCHLEVEL 184
#include "ruby/version.h" #include "ruby/version.h"
#include "ruby/internal/abi.h" #include "ruby/internal/abi.h"