Module#remove_const: cleanup vm->defined_module_hash#10143
Module#remove_const: cleanup vm->defined_module_hash#10143casperisfine wants to merge 4 commits intoruby:masterfrom
Conversation
Launchable Report❌ Test session #2670190 failed ❌ Test session #2670224 failed ❌ Test session #2670236 failed ❌ Test session #2670241 failed Passed test sessions✅ Test session #2670186 passed Build: refs_pull_10143_merge_96be457649dbb8f759159899d851a073753445fe |
ee97604 to
df5b171
Compare
[Bug #20311] All module and classes defined from C are stored in this table to ensure they are pinned and won't be moved by the compactor. However this means we need to remove them from it whenever a constant referencing them is removed. But since a same class or module may be referenced under multiple names, we have to do some ref counting.
df5b171 to
88bf9f5
Compare
|
looks like I introduced a bug that cause some classes not to be pinned. Trying to figure it out. |
|
Alright, so the initial patch would break because some code in Ruby rely on this bug. e.g. But that also means removing modules from the root set can't be done when they are de-referenced, it must be done when they are GCed. Currently my patch does that, but now the problem is that a lot of core, including inside Ruby itself rely on classes and module define from C being immortal. For the most part it's fine, but it means if their const is removed it then become trivial to crash the VM. So I'd need to add a whole lot of |
dd413bd to
2fac641
Compare
2fac641 to
96be457
Compare
|
Closing in favor of #10144 |
[Bug #20311]
All module and classes defined from C are stored in this table to ensure they are pinned and won't be moved by the compactor.
However this means we need to remove them from it whenever a constant referencing them is removed.
But since a same class or module may be referenced under multiple names, we have to do some ref counting.