Skip to content

Commit

Permalink
[rubygems/rubygems] Fix interrupt handling in Bundler workers
Browse files Browse the repository at this point in the history
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 ruby#4764.

rubygems/rubygems@b9f455d487
  • Loading branch information
haines authored and hsbt committed Jul 27, 2021
1 parent c8172d0 commit 705b1bd
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 9 deletions.
7 changes: 0 additions & 7 deletions lib/bundler/shared_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -145,13 +145,6 @@ def print_major_deprecations!
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 }
Expand Down
19 changes: 17 additions & 2 deletions lib/bundler/worker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def initialize(size, name, func)
@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
Expand Down Expand Up @@ -68,13 +68,16 @@ def apply_func(obj, i)
# 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
Expand All @@ -94,11 +97,23 @@ def create_threads
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
47 changes: 47 additions & 0 deletions spec/bundler/bundler/worker_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,51 @@
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

0 comments on commit 705b1bd

Please sign in to comment.