Improve handling of SIGINT.

This commit is contained in:
Markus Reiter 2020-12-17 15:45:50 +01:00
parent ecfd77ed62
commit 3db55d13d6
5 changed files with 78 additions and 37 deletions

View File

@ -32,14 +32,10 @@ module Homebrew
ruby_sys_args << "-e #{args.e}" if args.e ruby_sys_args << "-e #{args.e}" if args.e
ruby_sys_args += args.named ruby_sys_args += args.named
begin exec RUBY_PATH,
safe_system RUBY_PATH,
ENV["HOMEBREW_RUBY_WARNINGS"], ENV["HOMEBREW_RUBY_WARNINGS"],
"-I", $LOAD_PATH.join(File::PATH_SEPARATOR), "-I", $LOAD_PATH.join(File::PATH_SEPARATOR),
"-rglobal", "-rdev-cmd/irb", "-rglobal", "-rdev-cmd/irb",
*ruby_sys_args *ruby_sys_args
rescue ErrorDuringExecution => e
exit e.status.exitstatus
end
end end
end end

View File

@ -578,14 +578,24 @@ class ErrorDuringExecution < RuntimeError
@status = status @status = status
@output = output @output = output
exitstatus = if status.respond_to?(:exitstatus) exitstatus = case status
status.exitstatus when Integer
else
status status
else
status.exitstatus
end end
redacted_cmd = redact_secrets(cmd.shelljoin.gsub('\=', "="), secrets) redacted_cmd = redact_secrets(cmd.shelljoin.gsub('\=', "="), secrets)
s = +"Failure while executing; `#{redacted_cmd}` exited with #{exitstatus}."
reason = if exitstatus
"exited with #{exitstatus}"
elsif (uncaught_signal = status.termsig)
"was terminated by uncaught signal #{Signal.signame(uncaught_signal)}"
else
raise ArgumentError, "Status does neither have `exitstatus` nor `termsig`."
end
s = +"Failure while executing; `#{redacted_cmd}` #{reason}."
if Array(output).present? if Array(output).present?
format_output_line = lambda do |type_line| format_output_line = lambda do |type_line|

View File

@ -20,6 +20,8 @@ class SystemCommand
# Helper functions for calling {SystemCommand.run}. # Helper functions for calling {SystemCommand.run}.
module Mixin module Mixin
extend T::Sig
def system_command(*args) def system_command(*args)
T.unsafe(SystemCommand).run(*args) T.unsafe(SystemCommand).run(*args)
end end
@ -32,8 +34,6 @@ class SystemCommand
include Context include Context
extend Predicable extend Predicable
attr_reader :pid
def self.run(executable, **options) def self.run(executable, **options)
T.unsafe(self).new(executable, **options).run! T.unsafe(self).new(executable, **options).run!
end end
@ -44,7 +44,7 @@ class SystemCommand
sig { returns(SystemCommand::Result) } sig { returns(SystemCommand::Result) }
def run! def run!
puts redact_secrets(command.shelljoin.gsub('\=', "="), @secrets) if verbose? || debug? $stderr.puts redact_secrets(command.shelljoin.gsub('\=', "="), @secrets) if verbose? || debug?
@output = [] @output = []
@ -152,28 +152,42 @@ class SystemCommand
end end
end end
def each_output_line(&b) sig { params(block: T.proc.params(type: Symbol, line: String).void).void }
def each_output_line(&block)
executable, *args = command executable, *args = command
options = {
# Create a new process group so that we can send `SIGINT` from
# parent to child rather than the child receiving `SIGINT` directly.
pgroup: true,
}
options[:chdir] = chdir if chdir
raw_stdin, raw_stdout, raw_stderr, raw_wait_thr = pid = T.let(nil, T.nilable(Integer))
T.unsafe(Open3).popen3(env, [executable, executable], *args, **{ chdir: chdir }.compact) raw_stdin, raw_stdout, raw_stderr, raw_wait_thr = ignore_interrupts do
@pid = raw_wait_thr.pid T.unsafe(Open3).popen3(env, [executable, executable], *args, **options)
.tap { |*, wait_thr| pid = wait_thr.pid }
end
write_input_to(raw_stdin) write_input_to(raw_stdin)
raw_stdin.close_write raw_stdin.close_write
each_line_from [raw_stdout, raw_stderr], &b each_line_from [raw_stdout, raw_stderr], &block
@status = raw_wait_thr.value @status = raw_wait_thr.value
rescue Interrupt
Process.kill("INT", pid) if pid
raise Interrupt
rescue SystemCallError => e rescue SystemCallError => e
@status = $CHILD_STATUS @status = $CHILD_STATUS
@output << [:stderr, e.message] @output << [:stderr, e.message]
end end
sig { params(raw_stdin: IO).void }
def write_input_to(raw_stdin) def write_input_to(raw_stdin)
input.each(&raw_stdin.method(:write)) input.each(&raw_stdin.method(:write))
end end
def each_line_from(sources) sig { params(sources: T::Array[IO], _block: T.proc.params(type: Symbol, line: String).void).void }
def each_line_from(sources, &_block)
loop do loop do
readable_sources, = IO.select(sources) readable_sources, = IO.select(sources)

View File

@ -24,7 +24,11 @@ describe SystemCommand do
it "includes the given variables explicitly" do it "includes the given variables explicitly" do
expect(Open3) expect(Open3)
.to receive(:popen3) .to receive(:popen3)
.with(an_instance_of(Hash), ["/usr/bin/env", "/usr/bin/env"], "A=1", "B=2", "C=3", "env", *env_args, {}) .with(
an_instance_of(Hash), ["/usr/bin/env", "/usr/bin/env"], "A=1", "B=2", "C=3",
"env", *env_args,
pgroup: true
)
.and_call_original .and_call_original
command.run! command.run!
@ -49,8 +53,10 @@ describe SystemCommand do
it "includes the given variables explicitly" do it "includes the given variables explicitly" do
expect(Open3) expect(Open3)
.to receive(:popen3) .to receive(:popen3)
.with(an_instance_of(Hash), ["/usr/bin/sudo", "/usr/bin/sudo"], "-E", "--", .with(
"/usr/bin/env", "A=1", "B=2", "C=3", "env", *env_args, {}) an_instance_of(Hash), ["/usr/bin/sudo", "/usr/bin/sudo"], "-E", "--",
"/usr/bin/env", "A=1", "B=2", "C=3", "env", *env_args, pgroup: true
)
.and_wrap_original do |original_popen3, *_, &block| .and_wrap_original do |original_popen3, *_, &block|
original_popen3.call("true", &block) original_popen3.call("true", &block)
end end
@ -257,24 +263,22 @@ describe SystemCommand do
context "when given arguments with secrets" do context "when given arguments with secrets" do
it "does not leak the secrets" do it "does not leak the secrets" do
redacted_msg = /#{Regexp.escape("username:******")}/ redacted_msg = /#{Regexp.escape("username:******")}/
expect do expect {
described_class.run! "curl", described_class.run! "curl",
args: %w[--user username:hunter2], args: %w[--user username:hunter2],
verbose: true, verbose: true,
secrets: %w[hunter2] secrets: %w[hunter2]
end.to raise_error.with_message(redacted_msg).and output(redacted_msg).to_stdout }.to raise_error.with_message(redacted_msg).and output(redacted_msg).to_stderr
end end
it "does not leak the secrets set by environment" do it "does not leak the secrets set by environment" do
redacted_msg = /#{Regexp.escape("username:******")}/ redacted_msg = /#{Regexp.escape("username:******")}/
expect do expect {
ENV["PASSWORD"] = "hunter2" ENV["PASSWORD"] = "hunter2"
described_class.run! "curl", described_class.run! "curl",
args: %w[--user username:hunter2], args: %w[--user username:hunter2],
verbose: true verbose: true
ensure }.to raise_error.with_message(redacted_msg).and output(redacted_msg).to_stderr
ENV.delete "PASSWORD"
end.to raise_error.with_message(redacted_msg).and output(redacted_msg).to_stdout
end end
end end
end end

View File

@ -388,13 +388,30 @@ module Kernel
Pathname.new(cmd).archs Pathname.new(cmd).archs
end end
def ignore_interrupts(opt = nil) def ignore_interrupts(_opt = nil)
std_trap = trap("INT") do # rubocop:disable Style/GlobalVars
puts "One sec, just cleaning up" unless opt == :quietly $ignore_interrupts_nesting_level = 0 unless defined?($ignore_interrupts_nesting_level)
$ignore_interrupts_nesting_level += 1
$ignore_interrupts_interrupted = false unless defined?($ignore_interrupts_interrupted)
old_sigint_handler = trap(:INT) do
$ignore_interrupts_interrupted = true
$stderr.print "\n"
$stderr.puts "One sec, cleaning up..."
end end
begin
yield yield
ensure ensure
trap("INT", std_trap) trap(:INT, old_sigint_handler)
$ignore_interrupts_nesting_level -= 1
if $ignore_interrupts_nesting_level == 0 && $ignore_interrupts_interrupted
$ignore_interrupts_interrupted = false
raise Interrupt
end
end
# rubocop:enable Style/GlobalVars
end end
sig { returns(String) } sig { returns(String) }