Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

Commit

Permalink
Auto merge of #6223 - shayonj:s/hup-fix, r=segiddins
Browse files Browse the repository at this point in the history
Only trap INT signal and set to DEFAULT

### What was the end-user problem that led to this PR?

The problem was commands like `nohup bundler exec {program}` wouldn't work as intended. For  example, if a `HUP` signal were to be sent to the process running the `bundle exec ..`, it should in theory not terminate. Because, `nohup` would `IGNORE` that signal. But, that is not what the case is case is currently.

### What was your diagnosis of the problem?

My diagnosis was, if a process/bundler execution already had a `SIGNAL` set to it, example `HUP`, then performing `bundle exec {program}`, would mean that bundler resets any prior `SIGNAL`s on that process and sets them to `DEFAULT`.

### What is your fix for the problem, implemented in this PR?

My fix to the problem is to only trap `SIGNAL`s that we think should be set to `DEFAULT`, in this case, `INT`.

### Why did you choose this fix out of the possible options?

I chose this fix because its lot less aggressive than setting every signal to `DEFAULT`, and gives us the work with a smaller set of `SIGNAL`s. It also felt cleaner than having to trap a signal first and then restore to its predecessor value.

----

This is a dump that shows the before and after signals, when `nohup bundle exec {program }` gets run.

```
SIGEXIT: Before Handler: (), Current Handler: (DEFAULT)
SIGHUP: Before Handler: (IGNORE), Current Handler: (DEFAULT)
SIGINT: Before Handler: (#<Proc:0x00007f8e100534f8@/Users/<>/.rvm/gems/ruby-2.4.2/gems/bundler-1.16.0/exe/bundle:5>), Current Handler: (DEFAULT)
SIGQUIT: Before Handler: (DEFAULT), Current Handler: (DEFAULT)
SIGTRAP: Before Handler: (SYSTEM_DEFAULT), Current Handler: (DEFAULT)
SIGABRT: Before Handler: (SYSTEM_DEFAULT), Current Handler: (DEFAULT)
SIGIOT: Before Handler: (SYSTEM_DEFAULT), Current Handler: (DEFAULT)
SIGEMT: Before Handler: (SYSTEM_DEFAULT), Current Handler: (DEFAULT)
SIGSYS: Before Handler: (), Current Handler: (DEFAULT)
SIGPIPE: Before Handler: (), Current Handler: (DEFAULT)
SIGALRM: Before Handler: (DEFAULT), Current Handler: (DEFAULT)
SIGTERM: Before Handler: (DEFAULT), Current Handler: (DEFAULT)
SIGURG: Before Handler: (SYSTEM_DEFAULT), Current Handler: (DEFAULT)
SIGTSTP: Before Handler: (SYSTEM_DEFAULT), Current Handler: (DEFAULT)
SIGCONT: Before Handler: (SYSTEM_DEFAULT), Current Handler: (DEFAULT)
SIGCHLD: Before Handler: (SYSTEM_DEFAULT), Current Handler: (DEFAULT)
SIGCLD: Before Handler: (SYSTEM_DEFAULT), Current Handler: (DEFAULT)
SIGTTIN: Before Handler: (SYSTEM_DEFAULT), Current Handler: (DEFAULT)
SIGTTOU: Before Handler: (SYSTEM_DEFAULT), Current Handler: (DEFAULT)
SIGIO: Before Handler: (SYSTEM_DEFAULT), Current Handler: (DEFAULT)
SIGXCPU: Before Handler: (SYSTEM_DEFAULT), Current Handler: (DEFAULT)
SIGXFSZ: Before Handler: (SYSTEM_DEFAULT), Current Handler: (DEFAULT)
SIGPROF: Before Handler: (SYSTEM_DEFAULT), Current Handler: (DEFAULT)
SIGWINCH: Before Handler: (SYSTEM_DEFAULT), Current Handler: (DEFAULT)
SIGUSR1: Before Handler: (DEFAULT), Current Handler: (DEFAULT)
SIGUSR2: Before Handler: (DEFAULT), Current Handler: (DEFAULT)
SIGINFO: Before Handler: (SYSTEM_DEFAULT), Current Handler: (DEFAULT)
```

From this, we can see only `INT` is being trapped by bundler
```
SIGINT: Before Handler: (#<Proc:0x00007f8e100534f8@/Users/<>/.rvm/gems/ruby-2.4.2/gems/bundler-1.16.0/exe/bundle:5>), Current Handler: (DEFAULT)
```

hence, the only one being restored back to `DEFAULT`

----

Issue: #6150

(cherry picked from commit 5cf764d)
  • Loading branch information
bundlerbot authored and colby-swandale committed Apr 11, 2018
1 parent 6005ca1 commit c278eac
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 23 deletions.
5 changes: 2 additions & 3 deletions lib/bundler/cli/exec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module Bundler
class CLI::Exec
attr_reader :options, :args, :cmd

RESERVED_SIGNALS = %w[SEGV BUS ILL FPE VTALRM KILL STOP].freeze
TRAPPED_SIGNALS = %w[INT].freeze

def initialize(options, args)
@options = options
Expand Down Expand Up @@ -70,8 +70,7 @@ def kernel_load(file, *args)
ui = Bundler.ui
Bundler.ui = nil
require "bundler/setup"
signals = Signal.list.keys - RESERVED_SIGNALS
signals.each {|s| trap(s, "DEFAULT") }
TRAPPED_SIGNALS.each {|s| trap(s, "DEFAULT") }
Kernel.load(file)
rescue SystemExit, SignalException
raise
Expand Down
70 changes: 50 additions & 20 deletions spec/commands/exec_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -700,30 +700,60 @@ def bin_path(a,b,c)
end
end

context "signals being trapped by bundler" do
let(:executable) { strip_whitespace <<-RUBY }
#{shebang}
begin
Thread.new do
puts 'Started' # For process sync
STDOUT.flush
sleep 1 # ignore quality_spec
raise "Didn't receive INT at all"
end.join
rescue Interrupt
puts "foo"
end
RUBY
context "signal handling" do
let(:test_signals) do
open3_reserved_signals = %w[CHLD CLD PIPE]
reserved_signals = %w[SEGV BUS ILL FPE VTALRM KILL STOP EXIT]
bundler_signals = %w[INT]

Signal.list.keys - (bundler_signals + reserved_signals + open3_reserved_signals)
end

context "signals being trapped by bundler" do
let(:executable) { strip_whitespace <<-RUBY }
#{shebang}
begin
Thread.new do
puts 'Started' # For process sync
STDOUT.flush
sleep 1 # ignore quality_spec
raise "Didn't receive INT at all"
end.join
rescue Interrupt
puts "foo"
end
RUBY

it "receives the signal" do
skip "popen3 doesn't provide a way to get pid " unless RUBY_VERSION >= "1.9.3"
it "receives the signal", :ruby => ">= 1.9.3" do
bundle!("exec #{path}") do |_, o, thr|
o.gets # Consumes 'Started' and ensures that thread has started
Process.kill("INT", thr.pid)
end

bundle("exec #{path}") do |_, o, thr|
o.gets # Consumes 'Started' and ensures that thread has started
Process.kill("INT", thr.pid)
expect(out).to eq("foo")
end
end

expect(out).to eq("foo")
context "signals not being trapped by bunder" do
let(:executable) { strip_whitespace <<-RUBY }
#{shebang}
signals = #{test_signals.inspect}
result = signals.map do |sig|
Signal.trap(sig, "IGNORE")
end
puts result.select { |ret| ret == "IGNORE" }.count
RUBY

it "makes sure no unexpected signals are restored to DEFAULT" do
test_signals.each do |n|
Signal.trap(n, "IGNORE")
end

bundle!("exec #{path}")

expect(out).to eq(test_signals.count.to_s)
end
end
end
end
Expand Down

0 comments on commit c278eac

Please sign in to comment.