From: "jeremyevans0 (Jeremy Evans) via ruby-core" <ruby-core@...>
Date: 2023-06-12T02:01:36+00:00
Subject: [ruby-core:113875] [Ruby master Bug#19701] The rb_classext_t::classpath field is not marked for T_ICLASS

Issue #19701 has been updated by jeremyevans0 (Jeremy Evans).


There is a third option, set "FrozenCore" as an fstring that doesn't get garbage collected (via `rb_gc_register_address` or something).  That reduces the cost to 1 object marking per major GC.  That seems to be the best option to me if the commit shouldn't be reverted.  As to whether the commit should be reverted, hopefully @nobu can answer that.

@wks can you provide example code that crashes Ruby with the current implementation?  It would be useful when committing a test/spec that fixes this issue.

----------------------------------------
Bug #19701: The rb_classext_t::classpath field is not marked for T_ICLASS
https://bugs.ruby-lang.org/issues/19701#change-103523

* Author: wks (Kunshan Wang)
* Status: Open
* Priority: Normal
* Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN
----------------------------------------
I am hacking Ruby to dump information about some objects, and I found that the `rb_classext_t::classpath` field for `T_ICLASS` objects sometimes contains dangling references to dead objects.

The commit https://github.com/ruby/ruby/commit/081cc4eb283cb01ddffb364397e5175dbfacab66 set the `classpath` field of `rb_mRubyVMFrozenCore` to a string "FrozenCore" so that it can be dumped using the `rb_dump_literal` function.

However, in `gc_mark_children`, if the `obj` is a `T_ICLASS`, the `RCLASS_EXT(obj)->classpath` will not be marked.  As a result, if `rb_mRubyVMFrozenCore` is the only object that holds a reference to the string "FrozenCore", the string will be considered garbage and reclaimed during a GC, and the `classpath` will contain a dangling pointer.

There are two solutions to this problem.  We can take *one* of the approaches below (not both).

1.  Let the GC mark the `classpath` field.  I drafted a pull request here: https://github.com/ruby/ruby/pull/7875
2.  Revert the commit https://github.com/ruby/ruby/commit/081cc4eb283cb01ddffb364397e5175dbfacab66

Marking the `classpath` field in GC will keep the `rb_dump_literal` function working.  For debug purposes, we can also use that field to identify what object a given `T_ICLASS` is.  Adding one marked field may make GC slower, but I don't think it will be observable because there are far less `T_ICLASS` objects than ordinary objects.

If we reverting the commit above, the `classpath` will always be blank for all `T_ICLASS` objects. (Question: How do we enforce it?)  It will also save some memory by keeping less strings alive.  However, currently, "FrozenCore" seems to be the only `T_OBJECT` that has its classpath set, and it may not result in significant memory saving.  I don't know what purpose the `rb_dump_literal` function originally served.  Maybe it is still important.  Maybe it is safe to remove now.

Which of the two approaches should we take?  It looks like each of them has its pros and cons.



-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/