
Issue #19473 has been updated by Eregon (Benoit Daloze). OK, thank you all 3 for clarifying. I understand that point of view, even though I don't fully agree with it. One problem is this limitation makes it impossible to support `Timeout.timeout` in `trap`, see https://github.com/ruby/timeout/issues/17, but that would work 100% correctly without this limitation. In fact it already works correctly on JRuby and TruffleRuby which don't raise for case 1. I see two ways to resolve this: * Run signal handlers not on the main thread but another thread. Then there is no need to prevent Mutex/Monitor/etc in `trap` since there won't be such problems. It seems the best solution and already e.g. what the JVM does. It might have some incompatibility due to running that code on another thread, but it shouldn't be the case since even currently trap handlers run "concurrently" to the main thread (they can run at any interrupt point which is pretty much anywhere). For exceptions from trap handlers (like the default SIGINT handler), they can simply be forwarded to the main thread so that case is easy to stay compatible. I think the main thing missing here is someone taking the time to implement this, anyone interested? * Allow specific Mutex & Monitor to be marked as "trap-safe" via a new method (like `rb_mutex_allow_trap()` but as a Ruby method) as proposed by @ioquatix in https://bugs.ruby-lang.org/issues/19473#note-18. That method documentation can then show the examples above and examples of safe usages of it. The default is still safe, but then at least it's possible to use Mutex in trap handler when we have checked the logic would be correct and CRuby no longer prevents such correct cases. @ko1 @mame @tompng Any opinion on which of these two we should do? ---------------------------------------- Bug #19473: can't be called from trap context (ThreadError) is too limiting https://bugs.ruby-lang.org/issues/19473#change-113964 * Author: Eregon (Benoit Daloze) * Status: Open * ruby -v: ruby 3.2.1 (2023-02-08 revision 31819e82c8) [x86_64-linux] * Backport: 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN ---------------------------------------- Simple reproducer: ``` $ ruby -ve 'm=Mutex.new; trap(:HUP) { m.synchronize { p :OK } }; Process.kill :HUP, Process.pid; sleep 0.1' ruby 3.2.1 (2023-02-08 revision 31819e82c8) [x86_64-linux] -e:1:in `synchronize': can't be called from trap context (ThreadError) from -e:1:in `block in <main>' from -e:1:in `kill' from -e:1:in `<main>' ``` Expected behavior: ``` $ ruby -ve 'm=Mutex.new; trap(:HUP) { m.synchronize { p :OK } }; Process.kill :HUP, Process.pid; sleep 0.1' truffleruby 22.3.1, like ruby 3.0.3, GraalVM CE Native [x86_64-linux] :OK $ ruby -ve 'm=Mutex.new; trap(:HUP) { m.synchronize { p :OK } }; Process.kill :HUP, Process.pid; sleep 0.1' jruby 9.4.0.0 (3.1.0) 2022-11-23 95c0ec159f OpenJDK 64-Bit Server VM 17.0.6+10 on 17.0.6+10 +jit [x86_64-linux] :OK ``` This exception is highly problematic, for instance it breaks `Timeout.timeout` in `trap`: https://github.com/ruby/timeout/issues/17#issuecomment-1142035939 I suppose this behavior is because *sometimes* it's problematic to lock a Mutex in trap, e.g., if it's already locked by the main thread/fiber. But that would otherwise already raise `deadlock; recursive locking (ThreadError)`, so there is no point to fail early. And that's just one case, not all, so we should not always raise an exception. There seems to be no valid reason to prevent *all* `Mutex#synchronize` in `trap`. After all, if the Mutex for instance is only used in `trap`, it's well-defined AFAIK. For instance a given trap handler does not seem executed concurrently: ``` $ ruby -ve 'trap(:HUP) { puts "in trap\n"+caller.join("\n")+"\n\n"; sleep 0.1 }; pid = Process.pid; Process.wait fork { 20.times { Process.kill :HUP, pid } }; sleep 1' ruby 3.2.1 (2023-02-08 revision 31819e82c8) [x86_64-linux] in trap -e:1:in `wait' -e:1:in `<main>' in trap -e:1:in `wait' -e:1:in `<main>' in trap -e:1:in `wait' -e:1:in `<main>' in trap -e:1:in `wait' -e:1:in `<main>' in trap -e:1:in `wait' -e:1:in `<main>' in trap -e:1:in `wait' -e:1:in `<main>' ``` And if the trap handler using the Mutex is never called while the Mutex is held by the main thread/fiber, there is also no problem. -- https://bugs.ruby-lang.org/