From: "Eregon (Benoit Daloze) via ruby-core" <ruby-core@...>
Date: 2023-03-13T15:41:35+00:00
Subject: [ruby-core:112864] [Ruby master Bug#19473] can't be called from trap context (ThreadError) is too limiting

Issue #19473 has been updated by Eregon (Benoit Daloze).


ko1 (Koichi Sasada) wrote in #note-13:
> I think this reason violates (1).

What do you mean?
AFAIK https://github.com/ruby/timeout/issues/17#issuecomment-1464039853 is correct.
If it's incorrect, please point the mistake in my reasoning.

We can't have understanding `(1)` by using solution `1.` (remove the trap+Mutex check), indeed those are exclusive.
IMO it's still worth it because we are always breaking when it's only rarely a problem.
So CRuby is causing hard failures for things that might just work.

Here is a list of your 3 understandings and how the 3 solutions address them, I also add (0):
(0) Fixes soundness, i.e. doesn't fail when the code correctly handles signal handler reentrancy

Solutions:
1. fixes (0)
2. fixes (0), (1) because no change there, (2) because opt-in
3. fixes (0), (1), (2), (3)

----------------------------------------
Bug #19473: can't be called from trap context (ThreadError) is too limiting
https://bugs.ruby-lang.org/issues/19473#change-102380

* Author: Eregon (Benoit Daloze)
* Status: Open
* Priority: Normal
* 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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/