From 705b1bdef2caf1bee052384b3c62c201f4fa5479 Mon Sep 17 00:00:00 2001
From: Andrew Haines <andrew@haines.org.nz>
Date: Thu, 15 Jul 2021 11:36:46 +0100
Subject: [PATCH] [rubygems/rubygems] Fix interrupt handling in Bundler workers

The existing interrupt handling using `SharedHelpers.trap` fails when the previous
handler for a signal is not callable (for example, when it is the string "DEFAULT").

Instead, we now handle interrupts by aborting the process when worker threads are
running, and restore the previous handler after worker threads are finished.

Fixes #4764.

https://github.com/rubygems/rubygems/commit/b9f455d487
---
 lib/bundler/shared_helpers.rb       |  7 -----
 lib/bundler/worker.rb               | 19 ++++++++++--
 spec/bundler/bundler/worker_spec.rb | 47 +++++++++++++++++++++++++++++
 3 files changed, 64 insertions(+), 9 deletions(-)

diff --git a/lib/bundler/shared_helpers.rb b/lib/bundler/shared_helpers.rb
index f43e3301ef..405ade95dd 100644
--- a/lib/bundler/shared_helpers.rb
+++ b/lib/bundler/shared_helpers.rb
@@ -145,13 +145,6 @@ module Bundler
       Bundler.ui.warn message
     end
 
-    def trap(signal, override = false, &block)
-      prior = Signal.trap(signal) do
-        block.call
-        prior.call unless override
-      end
-    end
-
     def ensure_same_dependencies(spec, old_deps, new_deps)
       new_deps = new_deps.reject {|d| d.type == :development }
       old_deps = old_deps.reject {|d| d.type == :development }
diff --git a/lib/bundler/worker.rb b/lib/bundler/worker.rb
index 0380096523..5e4ee21c51 100644
--- a/lib/bundler/worker.rb
+++ b/lib/bundler/worker.rb
@@ -26,7 +26,7 @@ module Bundler
       @func = func
       @size = size
       @threads = nil
-      SharedHelpers.trap("INT") { abort_threads }
+      @previous_interrupt_handler = nil
     end
 
     # Enqueue a request to be executed in the worker pool
@@ -68,13 +68,16 @@ module Bundler
     # so as worker threads after retrieving it, shut themselves down
     def stop_threads
       return unless @threads
+
       @threads.each { @request_queue.enq POISON }
       @threads.each(&:join)
+
+      remove_interrupt_handler
+
       @threads = nil
     end
 
     def abort_threads
-      return unless @threads
       Bundler.ui.debug("\n#{caller.join("\n")}")
       @threads.each(&:exit)
       exit 1
@@ -94,11 +97,23 @@ module Bundler
         end
       end.compact
 
+      add_interrupt_handler unless @threads.empty?
+
       return if creation_errors.empty?
 
       message = "Failed to create threads for the #{name} worker: #{creation_errors.map(&:to_s).uniq.join(", ")}"
       raise ThreadCreationError, message if @threads.empty?
       Bundler.ui.info message
     end
+
+    def add_interrupt_handler
+      @previous_interrupt_handler = trap("INT") { abort_threads }
+    end
+
+    def remove_interrupt_handler
+      return unless @previous_interrupt_handler
+
+      trap "INT", @previous_interrupt_handler
+    end
   end
 end
diff --git a/spec/bundler/bundler/worker_spec.rb b/spec/bundler/bundler/worker_spec.rb
index 2e5642709d..e4ebbd2932 100644
--- a/spec/bundler/bundler/worker_spec.rb
+++ b/spec/bundler/bundler/worker_spec.rb
@@ -19,4 +19,51 @@ RSpec.describe Bundler::Worker do
       end
     end
   end
+
+  describe "handling interrupts" do
+    let(:status) do
+      pid = Process.fork do
+        $stderr.reopen File.new("/dev/null", "w")
+        Signal.trap "INT", previous_interrupt_handler
+        subject.enq "a"
+        subject.stop unless interrupt_before_stopping
+        Process.kill "INT", Process.pid
+      end
+
+      Process.wait2(pid).last
+    end
+
+    before do
+      skip "requires Process.fork" unless Process.respond_to?(:fork)
+    end
+
+    context "when interrupted before stopping" do
+      let(:interrupt_before_stopping) { true }
+      let(:previous_interrupt_handler) { ->(*) { exit 0 } }
+
+      it "aborts" do
+        expect(status.exitstatus).to eq(1)
+      end
+    end
+
+    context "when interrupted after stopping" do
+      let(:interrupt_before_stopping) { false }
+
+      context "when the previous interrupt handler was the default" do
+        let(:previous_interrupt_handler) { "DEFAULT" }
+
+        it "uses the default interrupt handler" do
+          expect(status).to be_signaled
+        end
+      end
+
+      context "when the previous interrupt handler was customized" do
+        let(:previous_interrupt_handler) { ->(*) { exit 42 } }
+
+        it "restores the custom interrupt handler after stopping" do
+          expect(status.exitstatus).to eq(42)
+        end
+      end
+    end
+  end
 end