Issue #20169 has been updated by kjtsanaktsidis (KJ Tsanaktsidis).
IMO it's undefined behaviour to use a Ruby object in a non-GVL scenario, this just fixes the issue inside of Ruby using a hack.
In this case, shouldn’t we fix the original issuing by copying the write through a C-allocated buffer? But…
They're not exactly rules in CRuby
I guess what you’re saying is “CRuby itself can do what it wants as long as it’s internally consistent”. Which makes sense. I guess it would be good to find a way to define and enforce what “internally consistent” means, but perhaps there isn’t much value in strictly defining it in the absence of a way to strictly enforce it. Things like VM_CHECK_MODE help, but couldn’t catch something like this. If I actually come up with an actionable idea, I’ll open a new proposal for that. ——— Anyway, fixing this bug with your patch sounds good to me. I definitely am not trying to hold up real fixes just to have academic arguments about memory models. ---------------------------------------- Bug #20169: `GC.compact` can raises `EFAULT` on IO https://bugs.ruby-lang.org/issues/20169#change-106593 * Author: ko1 (Koichi Sasada) * Status: Open * Priority: Normal * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- 1. `GC.compact` introduces read barriers to detect read accesses to the pages. 2. I/O operations release GVL to pass the control while their execution, and another thread can call `GC.compact` (or auto compact feature I guess, but not checked yet). 3. Call `write(ptr)` can return `EFAULT` when `GC.compact` is running because `ptr` can point read-barrier protected pages (embed strings). Reproducible steps: Apply the following patch to increase possibility: ```patch diff --git a/io.c b/io.c index f6cd2c1a56..83d67ba2dc 100644 --- a/io.c +++ b/io.c @@ -1212,8 +1212,12 @@ internal_write_func(void *ptr) } } + int cnt = 0; retry: - do_write_retry(write(iis->fd, iis->buf, iis->capa)); + for (; cnt < 1000; cnt++) { + do_write_retry(write(iis->fd, iis->buf, iis->capa)); + if (result <= 0) break; + } if (result < 0 && !iis->nonblock) { int e = errno; ``` Run the following code: ```ruby t1 = Thread.new{ 10_000.times.map{"#{_1}"}; GC.compact while true } t2 = Thread.new{ i=0 $stdout.write "<#{i+=1}>" while true } t2.join ``` and ``` $ make run (snip) 4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4><4>#<Thread:0x00007fa61b4dd758 ../../src/trunk/test.rb:3 run> terminated with exception (report_on_exception is true): ../../src/trunk/test.rb:5:in `write': Bad address @ io_write - <STDOUT> (Errno::EFAULT) from ../../src/trunk/test.rb:5:in `block in <main>' ../../src/trunk/test.rb:5:in `write': Bad address @ io_write - <STDOUT> (Errno::EFAULT) from ../../src/trunk/test.rb:5:in `block in <main>' make: *** [uncommon.mk:1383: run] Error 1 ``` I think this is why we get `EFAULT` on CI. To increase possibilities running many busy processes (`ruby -e 'loop{}'` for example) will help (and on CI environment there are such busy processes accidentally). -- https://bugs.ruby-lang.org/