[Bug #21090]
The struct was previously allocated on the stack, which could be freed
if the Thread is terminated. Moving this to a T_DATA on the heap should
mean this is no longer an issue.
1000.times { Ractor.new { th = Thread.new { require "rbconfig" }; Thread.pass }.take }
Co-authored-by: Luke Gruber <luke.gruber@shopify.com>
Previously rb_ractor_interrupt_exec would use an intermediate function
to create a new thread with the actual target function, replacing the
data being passed in with a piece of malloc memory holding the "next"
function and the original data.
Because of this, passing rb_interrupt_exec_flag_value_data to
rb_ractor_interrupt_exec didn't have the intended effect of allowing
data to be passed in and marked.
This commit adds a rb_interrupt_exec_flag_new_thread flag, which
both simplifies the implementation and allows the original data to be
marked.
Using this `reg_cache` is racy across ractors, so don't use it when in a
ractor. Also, its use across ractors can cause a regular expression created
in 1 ractor to be used in another ractor (an isolation bug).
We never freed any resources of rb_native_thread at fork because it would
cause it to hang. This is because it called rb_native_cond_destroy for
condition variables. We can't call rb_native_cond_destroy here because
according to the specs of pthread_cond_destroy:
Attempting to destroy a condition variable upon which other threads
are currently blocked results in undefined behavior.
Specifically, glibc's pthread_cond_destroy waits on all the other listeners.
Since after forking all the threads are dead, the condition variable's
listeners will never wake up, so it will hang forever.
This commit changes it to only free the memory and none of the condition
variables.
[Bug #21439] Fix PM_SPLAT_NODE compilation error in for loops
This commit fixes a crash that occurred when using splat nodes (*) as
the index variable in for loops. The error "Unexpected node type for
index in for node: PM_SPLAT_NODE" was thrown because the compiler
didn't know how to handle splat nodes in this context.
The fix allows code like `for *x in [[1,2], [3,4]]` to compile and
execute correctly, where the splat collects each sub-array.
Now that class fields have been deletated to a T_IMEMO/class_fields
when we're in multi-ractor mode, we can read and write class instance
variable in an atomic way using Read-Copy-Update (RCU).
Note when in multi-ractor mode, we always use RCU. In theory
we don't need to, instead if we ensured the field is written
before the shape is updated it would be safe.
Benchmark:
```ruby
Warning[:experimental] = false
class Foo
@foo = 1
@bar = 2
@baz = 3
@egg = 4
@spam = 5
class << self
attr_reader :foo, :bar, :baz, :egg, :spam
end
end
ractors = 8.times.map do
Ractor.new do
1_000_000.times do
Foo.bar + Foo.baz * Foo.egg - Foo.spam
end
end
end
if Ractor.method_defined?(:value)
ractors.each(&:value)
else
ractors.each(&:take)
end
```
This branch vs Ruby 3.4:
```bash
$ hyperfine -w 1 'ruby --disable-all ../test.rb' './miniruby ../test.rb'
Benchmark 1: ruby --disable-all ../test.rb
Time (mean ± σ): 3.162 s ± 0.071 s [User: 2.783 s, System: 10.809 s]
Range (min … max): 3.093 s … 3.337 s 10 runs
Benchmark 2: ./miniruby ../test.rb
Time (mean ± σ): 208.7 ms ± 4.6 ms [User: 889.7 ms, System: 6.9 ms]
Range (min … max): 202.8 ms … 222.0 ms 14 runs
Summary
./miniruby ../test.rb ran
15.15 ± 0.47 times faster than ruby --disable-all ../test.rb
```
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.
The type isn't opaque because Ruby isn't often compiled with LTO,
so for optimization purpose it's better to allow as much inlining
as possible.
However ideally only `shape.c` and `shape.h` should deal with
the actual struct, and everything else should just deal with opaque
`shape_id_t`.
Previously, `asm.mov(m32, imm32)` panicked when `imm32 > 0x80000000`. It
attempted to split imm32 into a register before doing the store, but
then the register size didn't match the destination size.
Instead of splitting, use the `MOV r/m32, imm32` form which works for
all 32-bit values. Adjust asserts that assumed that all forms undergo
sign extension, which is not true for this case.
See: 54edc930f9f0a658da45cfcef46648d1b6f82467
Previously, `asm.mov(m32, imm32)` panicked when `imm32 > 0x80000000`. It
attempted to split imm32 into a register before doing the store, but
then the register size didn't match the destination size.
Instead of splitting, use the `MOV r/m32, imm32` form which works for
all 32-bit values. Adjust asserts that assumed that all forms undergo
sign extension, which is not true for this case.
See: 54edc930f9f0a658da45cfcef46648d1b6f82467
If `gem pristine foo` is run, and there's a default copy of foo, only
executables for it are reset. However, that was causing other copies of
`foo` to only reset executables, which is unexpected.
We should not modify `options[:only_executables]`, but respect its value
for every gem, and make sure special handling for default gems does not
leak to other gems.
https://github.com/rubygems/rubygems/commit/2c3039f1b0
In commit d42b9ffb206, an optimization was introduced that can speed up
Regexp#match by 15% when it matches with strings of different encodings.
This optimization, however, does not work across ractors. To fix this,
we only use the optimization if no ractors have been started. In the
future, we could use atomics for the reference counting if we find it's
needed and if it's more performant.
The backtrace of the misbehaving native thread:
```
* frame #0: 0x0000000189c94388 libsystem_kernel.dylib`__pthread_kill + 8
frame #1: 0x0000000189ccd88c libsystem_pthread.dylib`pthread_kill + 296
frame #2: 0x0000000189bd6c60 libsystem_c.dylib`abort + 124
frame #3: 0x0000000189adb174 libsystem_malloc.dylib`malloc_vreport + 892
frame #4: 0x0000000189adec90 libsystem_malloc.dylib`malloc_report + 64
frame #5: 0x0000000189ae321c libsystem_malloc.dylib`___BUG_IN_CLIENT_OF_LIBMALLOC_POINTER_BEING_FREED_WAS_NOT_ALLOCATED + 32
frame #6: 0x00000001001c3be4 ruby`onig_free_body(reg=0x000000012d84b660) at regcomp.c:5663:5
frame #7: 0x00000001001ba828 ruby`rb_reg_prepare_re(re=4748462304, str=4748451168) at re.c:1680:13
frame #8: 0x00000001001bac58 ruby`rb_reg_onig_match(re=4748462304, str=4748451168, match=(ruby`reg_onig_search [inlined] rbimpl_RB_TYPE_P_fastpath at value_type.h:349:14
ruby`reg_onig_search [inlined] rbimpl_rstring_getmem at rstring.h:391:5
ruby`reg_onig_search at re.c:1781:5), args=0x000000013824b168, regs=0x000000013824b150) at re.c:1708:20
frame #9: 0x00000001001baefc ruby`rb_reg_search_set_match(re=4748462304, str=4748451168, pos=<unavailable>, reverse=0, set_backref_str=1, set_match=0x0000000000000000) at re.c:1809:27
frame #10: 0x00000001001bae80 ruby`rb_reg_search0(re=<unavailable>, str=<unavailable>, pos=<unavailable>, reverse=<unavailable>, set_backref_str=<unavailable>, match=<unavailable>) at re.c:1861:12 [artificial]
frame #11: 0x0000000100230b90 ruby`rb_pat_search0(pat=<unavailable>, str=<unavailable>, pos=<unavailable>, set_backref_str=<unavailable>, match=<unavailable>) at string.c:6619:16 [artificial]
frame #12: 0x00000001002287f4 ruby`rb_str_sub_bang [inlined] rb_pat_search(pat=4748462304, str=4748451168, pos=0, set_backref_str=1) at string.c:6626:12
frame #13: 0x00000001002287dc ruby`rb_str_sub_bang(argc=1, argv=0x00000001381280d0, str=4748451168) at string.c:6668:11
frame #14: 0x000000010022826c ruby`rb_str_sub
```
You can reproduce this by running:
```
RUBY_TESTOPTS="--name=/test_str_capitalize/" make test-all TESTS=test/ruby/test_m17n.comb
```
However, you need to run it with multiple ractors at once.
Co-authored-by: jhawthorn <john@hawthorn.email>
