Skip to content

Conversation

kaorukobo
Copy link

Describe the change

Use Thread.start instead of Thread.new to avoid the can't alloc thread error under specific conditions.

Why are we doing this?

Under these conditions, the can't alloc thread error occurs:

  • Use Command#run in a non-main thread
  • The main thread waits for the thread using Command#run with Thread#join
  • An interrupt occurs on all threads while Command#run is executing
  • Use Command#run again in an error handler that processes the interrupt

Here is a reproduction example:

$ docker run -it --rm ruby:3.4-bullseye
$ gem install -N tty-command
$ ruby <<'RUBY'
require 'tty-command'

cmd = TTY::Command.new

th = Thread.start do
  begin
    cmd.run("echo running..., can you stop me with Ctrl+C?; sleep 9999")
  ensure
    cmd.run("echo cleanup..., can you stop me with Ctrl+C?; sleep 9999")
  end
end

th.join
RUBY

The result is as follows. The echo cleanup... is not executed:

[fddf6de0] Running echo running..., can you stop me with Ctrl+C?; sleep 9999
[fddf6de0]      running..., can you stop me with Ctrl+C?
^C[83236c90] Running echo cleanup..., can you stop me with Ctrl+C?; sleep 9999
#<Thread:0x00007fa203aaf3f0 -:5 aborting> terminated with exception (report_on_exception is true):
/usr/local/bundle/gems/tty-command-0.10.1/lib/tty/command/process_runner.rb:166:in 'Thread.new': can't alloc thread (ThreadError)
        from /usr/local/bundle/gems/tty-command-0.10.1/lib/tty/command/process_runner.rb:166:in 'TTY::Command::ProcessRunner#read_stream'
        from /usr/local/bundle/gems/tty-command-0.10.1/lib/tty/command/process_runner.rb:143:in 'TTY::Command::ProcessRunner#read_streams'
        from /usr/local/bundle/gems/tty-command-0.10.1/lib/tty/command/process_runner.rb:48:in 'TTY::Command::ProcessRunner#run!'
        from /usr/local/bundle/gems/tty-command-0.10.1/lib/tty/command.rb:185:in 'TTY::Command#execute_command'
        from /usr/local/bundle/gems/tty-command-0.10.1/lib/tty/command.rb:104:in 'TTY::Command#run'
        from -:9:in 'block in <main>'
-:13:in 'Thread#join': Interrupt
        from -:13:in '<main>'

It should work like this👇️: echo cleanup... should be executed and should also be stoppable with Ctrl+C.
(The verbose error messages from read_streams are problematic, but let's consider that a separate issue.)

[cead5bd7] Running echo running..., can you stop me with Ctrl+C?; sleep 9999
[cead5bd7]      running..., can you stop me with Ctrl+C?
^C[62d77c5a] Running echo cleanup..., can you stop me with Ctrl+C?; sleep 9999
[62d77c5a]      cleanup..., can you stop me with Ctrl+C?
^C/usr/local/bundle/gems/tty-command-0.10.1/lib/tty/command/process_runner.rb:146:in 'Thread#join': Interrupt
        from /usr/local/bundle/gems/tty-command-0.10.1/lib/tty/command/process_runner.rb:146:in 'TTY::Command::ProcessRunner#read_streams'
        from /usr/local/bundle/gems/tty-command-0.10.1/lib/tty/command/process_runner.rb:48:in 'TTY::Command::ProcessRunner#run!'
        from /usr/local/bundle/gems/tty-command-0.10.1/lib/tty/command.rb:185:in 'TTY::Command#execute_command'
        from /usr/local/bundle/gems/tty-command-0.10.1/lib/tty/command.rb:104:in 'TTY::Command#run'
        from -:9:in '<main>'
/usr/local/bundle/gems/tty-command-0.10.1/lib/tty/command/process_runner.rb:146:in 'Thread#join': Interrupt
        from /usr/local/bundle/gems/tty-command-0.10.1/lib/tty/command/process_runner.rb:146:in 'TTY::Command::ProcessRunner#read_streams'
        from /usr/local/bundle/gems/tty-command-0.10.1/lib/tty/command/process_runner.rb:48:in 'TTY::Command::ProcessRunner#run!'
        from /usr/local/bundle/gems/tty-command-0.10.1/lib/tty/command.rb:185:in 'TTY::Command#execute_command'
        from /usr/local/bundle/gems/tty-command-0.10.1/lib/tty/command.rb:104:in 'TTY::Command#run'
        from -:7:in '<main>'

Benefits

This fixes the error.

Drawbacks

All existing tests pass, so existing code should continue to work correctly.

Requirements

  • Tests written & passing locally? ==> testing this in rspec is hard / all the existing tests pass
  • Code style checked?
  • Rebased with master branch?
  • Documentation updated? ==> not applicable
  • Changelog updated?

Additional Notes

Thread.new and Thread.start should behave exactly the same way in how tty-command uses them. I don't understand why simply switching to start prevents the error from occurring. This is more of a workaround for a Ruby interpreter bug rather than a tty-command bug.

https://ruby-doc.org/core-2.5.9/Thread.html

start([args]*) {|args| block }
Basically the same as ::new. However, if class Thread is subclassed, then calling start in that subclass will not invoke the subclass's initialize method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant