When a Ractor is started, rather than modifying the cc table in place we
make a copy and CAS the table. When making the copy we previously
skipped cc_entries with an invalidated CME. However, that left the CCs
associated with that entry orphaned. CCs do not mark their CME, and the
CAS removed the table from the live set, so nothing invalidated the CC
and it ended up pointing at a garbage object.
Co-authored-by: Luke Gruber <luke.gru@gmail.com>
Currently we maintain the subclasses list for two separate purposes (we essentially have to different relationships we're putting into the same list):
1. On a T_MODULE, we track the T_ICLASSes created to include it into
other classes. Used for method invalidation and propagating includes
on the module that happen after it's been used
2. On a T_CLASS/T_ICLASS, we track the T_CLASS/T_ICLASS which are the
immediate children of the class. We use this for method invalidation,
some cvar things, and to iterate through subclasses.
Purpose 1 does not have any issues with box, the T_ICLASS always belongs
to one specific module and that's immutable. This list can be box-global
(always use the prime classext or hoist it out) and only needs to be
pruned during free. If we care about behaviour under a particular box
(ie. the propagating includes), we should look up the current box being
modified on the ICLASS itself.
Purpose 2 is more complicated. It currently tracks the immediate
children, the T_CLASS or T_ICLASS whose super points back. Because super
is per-box and is mutable (include/prepend insert ICLASSes into the
chain) we need to update the list on include/prepend, entries must be
per-box, and we can have multiple entries per-box. *I propose we
simplify this by no longer tracking the immediate subclass*, but instead
tracking the T_CLASS -> ... -> T_CLASS relationship, ie. the inverse of
rb_class_superclass. That relationship is the same across all boxes and
immutable after Class creation.
As a special case the ICLASS for refinements are also added to the
purpose 2 list (on T_CLASS). As those ICLASS do not chain to an eventual
leaf T_CLASS.
When we need to find the classes which have included a module, we can
use the module subclasses list to find the ICLASS and then use
RCLASS_INCLUDER. If we needed to iterate all T_ICLASS, we could then
walk up the CLASS_SUPER chain, but I didn't find anywhere we needed to
do that.
The cc_entries list associated with the invalidated CME can be deleted
from the table during GC, so it isn't safe to copy (and we shouldn't
copy it anyways, it's stale data).
This is easier to access as ec->ractor_id instead of pointer-chasing through
ec->thread->ractor->ractor_id
Co-authored-by: Luke Gruber <luke.gru@gmail.com>
Before this change, GC'ing any Ractor object caused you to lose all
enabled tracepoints across all ractors (even main). Now tracepoints are
ractor-local and this doesn't happen. Internal events are still global.
Fixes [Bug #19112]
This rewrites the class allocator search to be faster. Instead of using
RCLASS_SUPER, which is now even slower due to Box, we can scan the
superclasses list to find a class where the allocator is defined.
This also disallows allocating from an ICLASS. Previously I believe that
was only done for FrozenCore, and that was changed in
e596cf6e93.
Previously, this didn't work correctly, resulting in a
SystemStackError. This fixes the issue by finding the related
superclass method entry, and updating the orig_me in the
refinement method to point to the superclass method.
Fixes [Bug #21446]
When defining a bmethod, we recorded the current Ractor's object in the
method. However that was never marked and so could be GC'd and reused by
a future Ractor. Instead we can use the Ractor's id, which we expect to
be unique forever.
Co-authored-by: Luke Gruber <luke.gru@gmail.com>
to adopt strict shareable rule.
* (basically) shareable objects only refer shareable objects
* (exception) shareable objects can refere unshareable objects
but should not leak reference to unshareable objects to Ruby world
We cannot pass &ccs into rb_managed_id_table_lookup because rb_managed_id_table_lookup
takes in a VALUE*, so it violates strict aliasing in C. This causes segfaults
when compiling with LTO enabled.
Previously, ruby2_keywords could be used on a method or proc with
post arguments, but I don't think the behavior is desired:
```ruby
def a(*c, **kw) [c, kw] end
def b(*a, b) a(*a, b) end
ruby2_keywords(:b)
b({foo: 1}, bar: 1)
```
This changes ruby2_keywords to emit a warning and not set the
flag on a method/proc with post arguments.
While here, fix the ruby2_keywords specs for warnings, since they
weren't testing what they should be testing. They all warned
because the method didn't accept a rest argument, not because it
accepted a keyword or keyword rest argument.
We lock the VM to invalidate method entries. However, we do not lock the
VM to call methods, so it's possible that during a method call the method
entry gets invalidated. We only check that the method entry in the callcache
is not invalidated at the beginning of the method call, which makes it
possible to have race conditions. This causes crashes like:
vm_callinfo.h:421: Assertion Failed: vm_cc_cme:cc->klass != Qundef || !vm_cc_markable(cc)
vm_insnhelper.c:2200: Assertion Failed: vm_lookup_cc:!METHOD_ENTRY_INVALIDATED(vm_cc_cme(ccs_cc))
This commit adds a VM barrier to method cache invalidation to ensure that
other Ractors are stopped at a safe-point before invalidating the method
entry.
* Skip assertion when cc->klass is Qundef
* Invalidate CCs when cme is invalidated in marking
* Add additional assertions that CC references stay valid
Co-authored-by: Peter Zhu <peter@peterzhu.ca>
Previously we were issuing writebarriers for each cc, but were missing
the cme.
We also need to avoid it being possible to run GC after we've copied the
values into the allocated array, but before they're visible in the
object.
`rb_class_cc_entries` is little more than a `len` and `capa`.
Hence embedding the entries doesn't cost much extra copying and
saves a bit of memory and some pointer chasing.
Co-Authored-By: Étienne Barrié <etienne.barrie@gmail.com>
In multi-ractor mode, the `cc_tbl` mutations use the RCU pattern,
which allow lock-less reads.
Based on the assumption that invalidations and misses should be
increasingly rare as the process ages, locking on modification
isn't a big concern.
For now this doesn't change anything, but now that the table
is managed by GC, it opens the door to use RCU when in multi-ractor
mode, hence allow unsynchornized reads.
One of the biggest remaining contention point is `RClass.cc_table`.
The logical solution would be to turn it into a managed object, so
we can use an RCU strategy, given it's read heavy.
However, that's not currently possible because the table can't
be freed before the owning class, given the class free function
MUST go over all the CC entries to invalidate them.
However if the `CC->klass` reference is weak marked, then the
GC will take care of setting the reference to `Qundef`.
ZJIT: Support invalidating method redefinition
This commit adds support for the MethodRedefined invariant to be invalidated
when a method is redefined.
Changes:
- Added CME pointer to the MethodRedefined invariant in HIR
- Updated all places where MethodRedefined invariants are created to
include the CME pointer
- Added handling for MethodRedefined invariants in gen_patch_point to
call track_cme_assumption, which registers the patch point for
invalidation when rb_zjit_cme_invalidate is called
This ensures that when a method is redefined, all JIT code that
depends on that method will be properly invalidated.
[DOC] Make protected documentation more explicit about differences
Protected is a common source of confusion for devs
coming from different languages to Ruby. This
commit makes the documentation more explicit about
the differences, so that the use case for
protected is clearer.
These functions conflict with the planned C-API functions. Since they
deal with the underlying set_table pointers and not Set instances,
this seems like a more accurate name as well.
Fixes [Bug #21201]
This change addresses a performance regression where defining methods
inside `refine` blocks caused severe slowdowns. The issue was due to
`rb_clear_all_refinement_method_cache()` triggering a full object
space scan via `rb_objspace_each_objects` to find and invalidate
affected callcaches, which is very inefficient.
To fix this, I introduce `vm->cc_refinement_table` to track
callcaches related to refinements. This allows us to invalidate
only the necessary callcaches without scanning the entire heap,
resulting in significant performance improvement.
Now that we have a `set_table` implementation, we can
use it to track const caches and save some memory.
We could even save some more memory if `numtable` didn't
store a copy of the `hash` and instead recomputed it every
time, but this is a quick win.
This changes reference_count on rb_method_definition_struct into an
atomic.
Ractors can create additional references as part of `bind_call` or
(presumably) similar. Because this can be done inside Ractors, we should
use a lock or atomics so that we don't race and avoid incrementing.
Co-authored-by: wanabe <s.wanabe@gmail.com>
