Attach a box classext to its class before filling it in

rb_class_duplicate_classext builds the whole copy first and the caller only
then puts it in the class's table, so a GC in the middle collects the tables
it has made and marking the class afterwards walks into a freed object.  Put
the classext in the table first instead.  Marking it while it is still half
built is fine, since ZALLOC leaves every field zero.

    $ RUBY_BOX=1 ruby --disable-gems -e 'GC.stress = true; module Enumerable; def m; end; end'
    -e:1: [BUG] try to mark T_NONE object (obj: out-of-heap:0x..., parent: ... T_MODULE/Module Enumerable)

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Hiroshi SHIBATAandClaude Opus 5 committed 2026-09-24 16:29:51 +09:00
1 parent f7839b5228
commit 44fff96b0b
3 files changed
+24 -7
Notes: git 2026-09-24 07:30:26 +00:00

No files matched your search

+8 -4
View File
@@ -292,6 +292,9 @@ class_duplicate_iclass_classext(VALUE iclass, rb_classext_t *mod_ext, const rb_b
RCLASSEXT_BOX(ext) = box;
VM_ASSERT(FL_TEST_RAW(iclass, RCLASS_BOXABLE));
first_set = RCLASS_SET_BOX_CLASSEXT(iclass, box, ext);
RCLASSEXT_SUPER(ext) = RCLASSEXT_SUPER(src);
// See also: rb_include_class_new()
@@ -317,16 +320,13 @@ class_duplicate_iclass_classext(VALUE iclass, rb_classext_t *mod_ext, const rb_b
RCLASSEXT_SET_INCLUDER(ext, iclass, RCLASSEXT_INCLUDER(src));
VM_ASSERT(FL_TEST_RAW(iclass, RCLASS_BOXABLE));
first_set = RCLASS_SET_BOX_CLASSEXT(iclass, box, ext);
if (first_set) {
RCLASS_SET_PRIME_CLASSEXT_WRITABLE(iclass, false);
}
}
rb_classext_t *
rb_class_duplicate_classext(rb_classext_t *orig, VALUE klass, const rb_box_t *box)
rb_class_duplicate_classext(rb_classext_t *orig, VALUE klass, const rb_box_t *box, int *first_set)
{
VM_ASSERT(RB_TYPE_P(klass, T_CLASS) || RB_TYPE_P(klass, T_MODULE) || RB_TYPE_P(klass, T_ICLASS));
@@ -335,6 +335,10 @@ rb_class_duplicate_classext(rb_classext_t *orig, VALUE klass, const rb_box_t *bo
RCLASSEXT_BOX(ext) = box;
/* Everything made below is reachable only through this classext, so put
* it where the GC can find it before allocating any of it. */
*first_set = RCLASS_SET_BOX_CLASSEXT(klass, box, ext);
RCLASSEXT_SUPER(ext) = RCLASSEXT_SUPER(orig);
RCLASSEXT_M_TBL(ext) = duplicate_classext_m_tbl(RCLASSEXT_M_TBL(orig), klass, dup_iclass);
+2 -3
View File
@@ -291,7 +291,7 @@ RCLASS_SET_CLASSEXT_TBL(VALUE klass, st_table *tbl)
}
/* class.c */
rb_classext_t * rb_class_duplicate_classext(rb_classext_t *orig, VALUE obj, const rb_box_t *box);
rb_classext_t * rb_class_duplicate_classext(rb_classext_t *orig, VALUE obj, const rb_box_t *box, int *first_set);
void rb_class_ensure_writable(VALUE obj);
void rb_class_set_box_classext(VALUE obj, const rb_box_t *box, rb_classext_t *ext);
@@ -413,8 +413,7 @@ RCLASS_EXT_WRITABLE_LOOKUP(VALUE obj, const rb_box_t *box)
// re-check the classext is not created to avoid the multi-thread race
ext = RCLASS_EXT_TABLE_LOOKUP_INTERNAL(obj, box);
if (!ext) {
ext = rb_class_duplicate_classext(RCLASS_EXT_PRIME(obj), obj, box);
first_set = RCLASS_SET_BOX_CLASSEXT(obj, box, ext);
ext = rb_class_duplicate_classext(RCLASS_EXT_PRIME(obj), obj, box, &first_set);
if (first_set) {
// TODO: are there any case that a class/module become non-writable after its birthtime?
RCLASS_SET_PRIME_CLASSEXT_WRITABLE(obj, false);
+14
View File
@@ -1634,4 +1634,18 @@ class TestBox < Test::Unit::TestCase
assert_equal "42", BoxIsolatedProcTest::PROC.call(42)
end;
end
def test_builtin_module_copied_into_box_survives_gc_stress
assert_separately([ENV_ENABLE_BOX], __FILE__, __LINE__, "#{<<~"begin;"}\n#{<<~'end;'}", ignore_stderr: true)
begin;
GC.stress = true
module Enumerable
def _test_defined_in_main_box; end
end
GC.start
GC.stress = false
assert_include Enumerable.instance_methods(false), :_test_defined_in_main_box
end;
end
end