Auto merge of #6223 - shayonj:s/hup-fix, r=segiddins

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: https://github.com/bundler/bundler/issues/6150

(cherry picked from commit 5cf764d48b9c3a4299f39b6b1a7344665e2f3fd1)

2 files changed, 52 insertions, 23 deletions

diff --git a/lib/bundler/cli/exec.rb b/lib/bundler/cli/exec.rb
index 2fdc614fbb..d6ac453676 100644
--- a/lib/bundler/cli/exec.rb
+++ b/lib/bundler/cli/exec.rb
@@ -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
@@ -70,8 +70,7 @@ module Bundler
       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
diff --git a/spec/commands/exec_spec.rb b/spec/commands/exec_spec.rb
index af88f3f043..d094c1d578 100644
--- a/spec/commands/exec_spec.rb
+++ b/spec/commands/exec_spec.rb
@@ -700,30 +700,60 @@ __FILE__: #{path.to_s.inspect}
       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