mirror of
https://github.com/ruby/ruby.git
synced 2022-11-09 12:17:21 -05:00
[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
This commit is contained in:
parent
c8172d0b96
commit
705b1bdef2
3 changed files with 64 additions and 9 deletions
|
@ -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 }
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
Loading…
Add table
Reference in a new issue