From: "ko1 (Koichi Sasada) via ruby-core" <ruby-core@...>
Date: 2023-03-29T08:27:05+00:00
Subject: [ruby-core:113039] [Ruby master Bug#19557] Deadlock on STDOUT(ERR) lock on signal handler

Issue #19557 has been reported by ko1 (Koichi Sasada).

----------------------------------------
Bug #19557: Deadlock on STDOUT(ERR) lock on signal handler
https://bugs.ruby-lang.org/issues/19557

* Author: ko1 (Koichi Sasada)
* Status: Open
* Priority: Normal
* Backport: 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN
----------------------------------------
The following Ruby code produces `deadlock; recursive locking (ThreadError)`.
It means some IO operations (`puts`, ...) is not used on trap handlers safely.

```ruby
trap(:USR1){puts 'world'}
Thread.new{loop{Process.kill(:USR1, $$); sleep 0.5}}
loop{puts 'hello'}
```

```
...
hello
hello
hello
hello
hello
../../src/clean/test.rb:2:in `write': deadlock; recursive locking (ThreadError)
        from ../../src/clean/test.rb:2:in `puts'
        from ../../src/clean/test.rb:2:in `puts'
        from ../../src/clean/test.rb:2:in `block in <main>'
        from ../../src/clean/test.rb:4:in `write'
        from ../../src/clean/test.rb:4:in `puts'
        from ../../src/clean/test.rb:4:in `puts'
        from ../../src/clean/test.rb:4:in `block in <main>'
        from <internal:kernel>:187:in `loop'
        from ../../src/clean/test.rb:4:in `<main>'
```

## Reason

* `puts()` calls `rb_io_writev()`
* it calls `IO#write`
* it calls `io_write_m()`
* it calls `io_writev()`
* it calls `io_fwritev()`
* it calls `io_binwritev()`
* it calls `rb_mutex_synchronize()` with `io_binwritev_internal()`
  * STDOUT's `fptr->write_lock` is acquired here
* `io_binwritev_internal()` calls `rb_writev_internal()`
* it calls `rb_thread_io_blocking_region()` with `internal_writev_func()`

Here, `internal_writev_func()` can be interrupted by signals and if a trap handler is registered, call the trap handler (written in Ruby, `puts 'world'` in this case) and call `Kernel#puts()` and `fptr->write_lock` is already acquired -> `deadlock; recursive locking`.

## Ideas

I'm not sure why `fptr->write_lock` is needed, but if there is no internal consistency issue, we can make `fptr->write_lock` `nil` at least for STDOUT/ERR.

Another idea is, calling trap handlers (or other interruptible Ruby code such as finalizers and so on) after releasing the lock. But I'm not sure it is feasible.




-- 
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/