Previously we had an assertion that the method table was only set on
young objects, and a comment stating that was how it needed to be used.
I think that confused the complexity of the write barriers that may be
needed here.
* Setting an empty M_TBL never needs a write barrier
* T_CLASS and T_MODULE should always fire a write barrier to newly added
methods
* T_ICLASS only needs a write barrier to methods when
RCLASSEXT_ICLASS_IS_ORIGIN(x) && !RCLASSEXT_ICLASS_ORIGIN_SHARED_MTBL(x)
We shouldn't assume that the object being young is sufficient, because
we also need write barriers for incremental marking and it's unreliable.
When creating a new origin in ensure_origin, we need to fire a write
barrier after RCLASS_WRITE_ORIGIN. rb_class_set_super allocates, so GC
could happen there, either incrementally marking or promoting the newly
allocated class, and only after RCLASS_WRITE_ORIGIN will origin mark
object in the M_TBL.
The `FL_FREEZE` flag is redundant with `SHAPE_ID_FL_FROZEN`, so
ideally it should be eliminated in favor of the later.
Doing so would eliminate the risk of desync between the two, but
also solve the problem of the frozen status being global in namespace
context (See Bug #21330).
Even when namespaces are enabled, only a few core classes created
during init will eventually be namespaced.
For these it's OK to allocate a 320B slot to hold the extra namespace
stuff.
But for any class created post init, we know we'll never need the
namespace and we can fit in a 160B slot.
This behave almost exactly as a T_OBJECT, the layout is entirely
compatible.
This aims to solve two problems.
First, it solves the problem of namspaced classes having
a single `shape_id`. Now each namespaced classext
has an object that can hold the namespace specific
shape.
Second, it open the door to later make class instance variable
writes atomics, hence be able to read class variables
without locking the VM.
In the future, in multi-ractor mode, we can do the write
on a copy of the `fields_obj` and then atomically swap it.
Considerations:
- Right now the `RClass` shape_id is always synchronized,
but with namespace we should likely mark classes that have
multiple namespace with a specific shape flag.
MAX_IV_COUNT is a hint which determines the size of variable width
allocation we should use for a given class. We don't need to scope this
by namespace, if we end up with larger builtin objects on some
namespaces that isn't a user-visible problem, just extra memory use.
Similarly variation_count is used to track if a given object has had too
many branches in shapes it has used, and to use too_complex when that
happens. That's also just a hint, so we can use the same value across
namespaces without it being visible to users.
Previously variation_count was being incremented (written to) on the
RCLASS_EXT_READABLE ext, which seems incorrect if we wanted it to be
different across namespaces
Previously we used a flag to set whether a module was uninitialized.
When checked whether a class was initialized, we first had to check that
it had a non-zero superclass, as well as that it wasn't BasicObject.
With the advent of namespaces, RCLASS_SUPER is now an expensive
operation, and though we could just check for the prime superclass, we
might as well take this opportunity to use a flag so that we can perform
the initialized check with as few instructions as possible.
It's possible in the future that we could prevent uninitialized classes
from being available to the user, but currently there are a few ways to
do that.
If any other flags were passed other than type they were ignored, so we
might as well be more explicit that that's all this accepts. This also
fixes the incorrect (internal) documentation.
It also turns out type is always known in the caller, so I made it
explicit in the two places additional flags were being passed.
This makes `RBobject` `4B` larger on 32 bit systems
but simplifies the implementation a lot.
[Feature #21353]
Co-authored-by: Jean Boussier <byroot@ruby-lang.org>
Superclasses can't be modified by user code, so do not need namespace
indirection. For example Object.superclass is always BasicObject, no
matter what modules are included onto it.
`ext` is newly allocated so it shouldn't need an assertion. The class
ext (which is always from the module) that we're passing to
`class_duplicate_iclass_classext` could legitimately have instance
variables on it. We just want to avoid copying them.
The assertion was making this crash:
```
$ RUBY_NAMESPACE=1 ./miniruby -e1
```
When classes are booted, they should all be writeable unless namespaces
are enabled. This commit adds an assertion to ensure that classes are
writable.
By making `super_classdepth` `uint16_t`, classes and modules can
now fit in 160B slots again.
The downside of course is that before `super_classdepth` was large
enough we never had to care about overflow, as you couldn't
realistically create enough classes to ever go over it.
With this change, while it is stupid, you could realistically
create an ancestor chain containing 65k classes and modules.
The macro RCLASS_EXT() accesses the prime classext directly, but it can be
valid only in a limited situation when namespace is enabled.
So, to prevent using RCLASS_EXT() in the wrong way, rename the macro and
let the developer check it is ok to access the prime classext or not.
To make RClass size smaller, move flags of prime classext readable/writable to:
readable - use ns_classext_tbl is NULL or not (if NULL, it's readable)
writable - use FL_USER2 of RBasic flags
[Bug #18119]
When we create classes, it pushes the class to the subclass list of the
superclass. This access needs to be synchronized because multiple Ractors
may be creating classes with the same superclass, which would cause race
conditions and cause the linked list to be corrupted.
For example, we can reproduce with this script crashing:
workers = (0...8).map do
Ractor.new do
loop do
100.times.map { Class.new }
Ractor.yield nil
end
end
end
100.times { Ractor.select(*workers) }
With ASAN enabled, we can see that there are use-after-free errors:
==176013==ERROR: AddressSanitizer: heap-use-after-free on address 0x5030000974f0 at pc 0x62f9e56f892d bp 0x7a503f1ffd90 sp 0x7a503f1ffd88
WRITE of size 8 at 0x5030000974f0 thread T4
#0 0x62f9e56f892c in rb_class_remove_from_super_subclasses class.c:149:24
#1 0x62f9e58c9dd2 in rb_gc_obj_free gc.c:1262:9
#2 0x62f9e58f6e19 in gc_sweep_plane gc/default/default.c:3450:21
#3 0x62f9e58f686a in gc_sweep_page gc/default/default.c:3535:13
#4 0x62f9e58f12b4 in gc_sweep_step gc/default/default.c:3810:9
#5 0x62f9e58ed2a7 in gc_sweep gc/default/default.c:4058:13
#6 0x62f9e58fac93 in gc_start gc/default/default.c:6402:13
#7 0x62f9e58e8b69 in heap_prepare gc/default/default.c:2032:13
#8 0x62f9e58e8b69 in heap_next_free_page gc/default/default.c:2255:9
#9 0x62f9e58e8b69 in newobj_cache_miss gc/default/default.c:2362:38
...
0x5030000974f0 is located 16 bytes inside of 24-byte region [0x5030000974e0,0x5030000974f8)
freed by thread T4 here:
#0 0x62f9e562f28a in free (miniruby+0x1fd28a) (BuildId: 5ad6d9e7cec8318df6726ea5ce34d3c76d0d0233)
#1 0x62f9e58ca2ab in rb_gc_impl_free gc/default/default.c:8102:9
#2 0x62f9e58ca2ab in ruby_sized_xfree gc.c:5029:13
#3 0x62f9e58ca2ab in ruby_xfree gc.c:5040:5
#4 0x62f9e56f88e6 in rb_class_remove_from_super_subclasses class.c:152:9
#5 0x62f9e58c9dd2 in rb_gc_obj_free gc.c:1262:9
#6 0x62f9e58f6e19 in gc_sweep_plane gc/default/default.c:3450:21
#7 0x62f9e58f686a in gc_sweep_page gc/default/default.c:3535:13
#8 0x62f9e58f12b4 in gc_sweep_step gc/default/default.c:3810:9
#9 0x62f9e58ed2a7 in gc_sweep gc/default/default.c:4058:13
...
previously allocated by thread T5 here:
#0 0x62f9e562f70d in calloc (miniruby+0x1fd70d) (BuildId: 5ad6d9e7cec8318df6726ea5ce34d3c76d0d0233)
#1 0x62f9e58c8e1a in calloc1 gc/default/default.c:1472:12
#2 0x62f9e58c8e1a in rb_gc_impl_calloc gc/default/default.c:8138:5
#3 0x62f9e58c8e1a in ruby_xcalloc_body gc.c:4964:12
#4 0x62f9e58c8e1a in ruby_xcalloc gc.c:4958:34
#5 0x62f9e56f906e in push_subclass_entry_to_list class.c:88:13
#6 0x62f9e56f906e in rb_class_subclass_add class.c:111:38
#7 0x62f9e56f906e in RCLASS_SET_SUPER internal/class.h:257:9
#8 0x62f9e56fca7a in make_metaclass class.c:786:5
#9 0x62f9e59db982 in rb_class_initialize object.c:2101:5
Ivars will longer be the only thing stored inline
via shapes, so keeping the `iv_index` and `ivptr` names
would be confusing.
Instance variables won't be the only thing stored inline
via shapes, so keeping the `ivptr` name would be confusing.
`field` encompass anything that can be stored in a VALUE array.
Similarly, `gen_ivtbl` becomes `gen_fields_tbl`.
Originally, if a class was defined with the class keyword, the cref had a
const_added callback, and the superclass an inherited callback, const_added was
called first, and inherited second.
This was discussed in
https://bugs.ruby-lang.org/issues/21143
and an attempt at changing this order was made.
While both constant assignment and inheritance have happened before these
callbacks are invoked, it was deemed nice to have the same order as in
C = Class.new
This was mostly for alignment: In that last use case things happen at different
times and therefore the order of execution is kind of obvious, whereas when the
class keyword is involved, the order is opaque to the user and it is up to the
interpreter.
However, soon in
https://bugs.ruby-lang.org/issues/21193
Matz decided to play safe and keep the existing order.
This reverts commits:
de097fbe5fde48e47ddf
[Misc #21143]
[Bug #21193]
The previous change caused a backward compatibility issue with code
that called `Object.const_source_location` from the `inherited` callback.
To fix this, the order is now:
- Define the constant
- Invoke `inherited`
- Invoke `const_set`
[Misc #21143]
Conceptually this makes sense and is more consistent with using
the `Name = Class.new(Superclass)` alternative method.
However the new class is still named before `inherited` is called.
Fix by always adding the generated iclass to the subclasses list,
otherwise the method cache for the iclass is not cleared when
the method in the module is overwritten.
Fixes [Bug #20716]
Previously it would bypass the `FL_ABLE` check, but
since shapes introduction, it started having a different
behavior than `OBJ_FREEZE`, as it would onyl set the `FL_FREEZE`
flag, but not update the shape.
I have no indication of this causing a bug yet, but it seems
like a trap waiting to happen.
[Feature #20205]
As a path toward enabling frozen string literals by default in the future,
this commit introduce "chilled strings". From a user perspective chilled
strings pretend to be frozen, but on the first attempt to mutate them,
they lose their frozen status and emit a warning rather than to raise a
`FrozenError`.
Implementation wise, `rb_compile_option_struct.frozen_string_literal` is
no longer a boolean but a tri-state of `enabled/disabled/unset`.
When code is compiled with frozen string literals neither explictly enabled
or disabled, string literals are compiled with a new `putchilledstring`
instruction. This instruction is identical to `putstring` except it marks
the String with the `STR_CHILLED (FL_USER3)` and `FL_FREEZE` flags.
Chilled strings have the `FL_FREEZE` flag as to minimize the need to check
for chilled strings across the codebase, and to improve compatibility with
C extensions.
Notes:
- `String#freeze`: clears the chilled flag.
- `String#-@`: acts as if the string was mutable.
- `String#+@`: acts as if the string was mutable.
- `String#clone`: copies the chilled flag.
Co-authored-by: Jean Boussier <byroot@ruby-lang.org>
This `st_table` is used to both mark and pin classes
defined from the C API. But `vm->mark_object_ary` already
does both much more efficiently.
Currently a Ruby process starts with 252 rooted classes,
which uses `7224B` in an `st_table` or `2016B` in an `RArray`.
So a baseline of 5kB saved, but since `mark_object_ary` is
preallocated with `1024` slots but only use `405` of them,
it's a net `7kB` save.
`vm->mark_object_ary` is also being refactored.
Prior to this changes, `mark_object_ary` was a regular `RArray`, but
since this allows for references to be moved, it was marked a second
time from `rb_vm_mark()` to pin these objects.
This has the detrimental effect of marking these references on every
minors even though it's a mostly append only list.
But using a custom TypedData we can save from having to mark
all the references on minor GC runs.
Addtionally, immediate values are now ignored and not appended
to `vm->mark_object_ary` as it's just wasted space.
This frees FL_USER0 on both T_MODULE and T_CLASS.
Note: prior to this, FL_SINGLETON was never set on T_MODULE,
so checking for `FL_SINGLETON` without first checking that
`FL_TYPE` was `T_CLASS` was valid. That's no longer the case.
[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.
We should set the m_tbl right after allocation before anything that can
trigger GC to avoid clone_p from becoming old and needing to fire write
barriers.
Co-authored-by: Aaron Patterson <tenderlove@ruby-lang.org>
