From bcded854ce13085f0c3b65fc7ba023f28768abee Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sun, 14 Jul 2024 11:41:38 -0400 Subject: [PATCH 1/9] Make `ignore_interrupts` thread-safe. --- Library/Homebrew/extend/kernel.rb | 44 +++++++++++++++++-------------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/Library/Homebrew/extend/kernel.rb b/Library/Homebrew/extend/kernel.rb index df6d227fdb..cc4f82d6a7 100644 --- a/Library/Homebrew/extend/kernel.rb +++ b/Library/Homebrew/extend/kernel.rb @@ -349,30 +349,34 @@ module Kernel end end + IGNORE_INTERRUPTS_MUTEX = Thread::Mutex.new.freeze + def ignore_interrupts(_opt = nil) - # rubocop:disable Style/GlobalVars - $ignore_interrupts_nesting_level = 0 unless defined?($ignore_interrupts_nesting_level) - $ignore_interrupts_nesting_level += 1 + IGNORE_INTERRUPTS_MUTEX.synchronize do + # rubocop:disable Style/GlobalVars + $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 - - begin - yield - ensure - 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 + $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 + + begin + yield + ensure + 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 - # rubocop:enable Style/GlobalVars end def redirect_stdout(file) From 1b79e01c5b803fd47c1ac54cf99fe032b458f817 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sun, 14 Jul 2024 11:42:06 -0400 Subject: [PATCH 2/9] Remove useless `ignore_interrupts`. --- Library/Homebrew/download_strategy.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb index f7db621abf..7f9f275d0e 100644 --- a/Library/Homebrew/download_strategy.rb +++ b/Library/Homebrew/download_strategy.rb @@ -433,13 +433,11 @@ class CurlDownloadStrategy < AbstractFileDownloadStrategy rescue ErrorDuringExecution raise CurlDownloadStrategyError, url end - ignore_interrupts do - cached_location.dirname.mkpath - temporary_path.rename(cached_location) - symlink_location.dirname.mkpath - end + cached_location.dirname.mkpath + temporary_path.rename(cached_location) end + symlink_location.dirname.mkpath FileUtils.ln_s cached_location.relative_path_from(symlink_location.dirname), symlink_location, force: true rescue CurlDownloadStrategyError raise if urls.empty? From 3c8497647e56dba1e2615481f283ce70355c4fbd Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sun, 14 Jul 2024 13:18:33 -0400 Subject: [PATCH 3/9] Fix unused argument. --- Library/Homebrew/extend/kernel.rb | 9 ++++++--- Library/Homebrew/utils/fork.rb | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/Library/Homebrew/extend/kernel.rb b/Library/Homebrew/extend/kernel.rb index cc4f82d6a7..0fed83aafd 100644 --- a/Library/Homebrew/extend/kernel.rb +++ b/Library/Homebrew/extend/kernel.rb @@ -351,7 +351,7 @@ module Kernel IGNORE_INTERRUPTS_MUTEX = Thread::Mutex.new.freeze - def ignore_interrupts(_opt = nil) + def ignore_interrupts(quiet: false) IGNORE_INTERRUPTS_MUTEX.synchronize do # rubocop:disable Style/GlobalVars $ignore_interrupts_nesting_level = 0 unless defined?($ignore_interrupts_nesting_level) @@ -360,8 +360,11 @@ module Kernel $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..." + + unless quiet + $stderr.print "\n" + $stderr.puts "One sec, cleaning up..." + end end begin diff --git a/Library/Homebrew/utils/fork.rb b/Library/Homebrew/utils/fork.rb index 7334225321..117b680ff9 100644 --- a/Library/Homebrew/utils/fork.rb +++ b/Library/Homebrew/utils/fork.rb @@ -75,7 +75,7 @@ module Utils exit!(true) end - ignore_interrupts(:quietly) do # the child will receive the interrupt and marshal it back + ignore_interrupts(quiet: true) do # the child will receive the interrupt and marshal it back begin socket = server.accept_nonblock rescue Errno::EAGAIN, Errno::EWOULDBLOCK, Errno::ECONNABORTED, Errno::EPROTO, Errno::EINTR From 27a70a40c2f564309f59caa6e29b29d11e392aea Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sun, 14 Jul 2024 13:36:43 -0400 Subject: [PATCH 4/9] Disallow nested `ignore_interrupts`. --- Library/Homebrew/extend/kernel.rb | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/Library/Homebrew/extend/kernel.rb b/Library/Homebrew/extend/kernel.rb index 0fed83aafd..e94a33f919 100644 --- a/Library/Homebrew/extend/kernel.rb +++ b/Library/Homebrew/extend/kernel.rb @@ -353,13 +353,9 @@ module Kernel def ignore_interrupts(quiet: false) IGNORE_INTERRUPTS_MUTEX.synchronize do - # rubocop:disable Style/GlobalVars - $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) + interrupted = false old_sigint_handler = trap(:INT) do - $ignore_interrupts_interrupted = true + interrupted = true unless quiet $stderr.print "\n" @@ -372,13 +368,8 @@ module Kernel ensure 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 + raise Interrupt if interrupted end - # rubocop:enable Style/GlobalVars end end From 6f58bffc5cd61a3ae990af87d12c1b92902432cc Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sun, 14 Jul 2024 13:48:23 -0400 Subject: [PATCH 5/9] Remove useless `ignore_interrupts`. --- Library/Homebrew/utils/github/artifacts.rb | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/Library/Homebrew/utils/github/artifacts.rb b/Library/Homebrew/utils/github/artifacts.rb index 70290e579e..9e0756a5f1 100644 --- a/Library/Homebrew/utils/github/artifacts.rb +++ b/Library/Homebrew/utils/github/artifacts.rb @@ -43,12 +43,11 @@ class GitHubArtifactDownloadStrategy < AbstractFileDownloadStrategy rescue ErrorDuringExecution raise CurlDownloadStrategyError, url end - ignore_interrupts do - cached_location.dirname.mkpath - temporary_path.rename(cached_location) - symlink_location.dirname.mkpath - end + cached_location.dirname.mkpath + temporary_path.rename(cached_location) end + + symlink_location.dirname.mkpath FileUtils.ln_s cached_location.relative_path_from(symlink_location.dirname), symlink_location, force: true end From 44766945fc0623a02d1f012694a0c3a0d0f3343f Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sun, 14 Jul 2024 14:32:17 -0400 Subject: [PATCH 6/9] Fix variable type. --- Library/Homebrew/extend/kernel.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/extend/kernel.rb b/Library/Homebrew/extend/kernel.rb index e94a33f919..79f566b0f0 100644 --- a/Library/Homebrew/extend/kernel.rb +++ b/Library/Homebrew/extend/kernel.rb @@ -353,7 +353,7 @@ module Kernel def ignore_interrupts(quiet: false) IGNORE_INTERRUPTS_MUTEX.synchronize do - interrupted = false + interrupted = T.let(false, T::Boolean) old_sigint_handler = trap(:INT) do interrupted = true From 447216a9604e79afac0392ad95d8e83142eeb0a7 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sun, 14 Jul 2024 14:32:49 -0400 Subject: [PATCH 7/9] Avoid `ignore_interrupts` in `safe_fork`. --- Library/Homebrew/utils/fork.rb | 39 ++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/Library/Homebrew/utils/fork.rb b/Library/Homebrew/utils/fork.rb index 117b680ff9..d37dda94ef 100644 --- a/Library/Homebrew/utils/fork.rb +++ b/Library/Homebrew/utils/fork.rb @@ -75,11 +75,13 @@ module Utils exit!(true) end - ignore_interrupts(quiet: true) do # the child will receive the interrupt and marshal it back + pid = T.must(pid) + + begin begin socket = server.accept_nonblock rescue Errno::EAGAIN, Errno::EWOULDBLOCK, Errno::ECONNABORTED, Errno::EPROTO, Errno::EINTR - retry unless Process.waitpid(T.must(pid), Process::WNOHANG) + retry unless Process.waitpid(pid, Process::WNOHANG) else socket.send_io(write) socket.close @@ -87,23 +89,24 @@ module Utils write.close data = read.read read.close - Process.wait(T.must(pid)) unless socket.nil? - - # 130 is the exit status for a process interrupted via Ctrl-C. - # We handle it here because of the possibility of an interrupted process terminating - # without writing its Interrupt exception to the error pipe. - raise Interrupt if $CHILD_STATUS.exitstatus == 130 - - if data.present? - error_hash = JSON.parse(T.must(data.lines.first)) - - e = ChildProcessError.new(error_hash) - - raise rewrite_child_error(e) - end - - raise "Forked child process failed: #{$CHILD_STATUS}" unless $CHILD_STATUS.success? + Process.waitpid(pid) unless socket.nil? + rescue Interrupt + Process.waitpid(pid) end + + # 130 is the exit status for a process interrupted via Ctrl-C. + raise Interrupt if $CHILD_STATUS.exitstatus == 130 + raise Interrupt if $CHILD_STATUS.termsig == Signal.list["INT"] + + if data.present? + error_hash = JSON.parse(T.must(data.lines.first)) + + e = ChildProcessError.new(error_hash) + + raise rewrite_child_error(e) + end + + raise "Forked child process failed: #{$CHILD_STATUS}" unless $CHILD_STATUS.success? end end end From 78d18354b6a3587c8e53ea9525f3e9cb54ecdf4b Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sun, 14 Jul 2024 14:42:12 -0400 Subject: [PATCH 8/9] Remove `ignore_interrupts` in `SystemCommand`. --- Library/Homebrew/system_command.rb | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/Library/Homebrew/system_command.rb b/Library/Homebrew/system_command.rb index 061cb1b90a..31915d1f5a 100644 --- a/Library/Homebrew/system_command.rb +++ b/Library/Homebrew/system_command.rb @@ -238,14 +238,12 @@ class SystemCommand } options[:chdir] = chdir if chdir - raw_stdin, raw_stdout, raw_stderr, raw_wait_thr = ignore_interrupts do - Open3.popen3( - env.merge({ "COLUMNS" => Tty.width.to_s }), - [executable, executable], - *args, - **options, - ) - end + raw_stdin, raw_stdout, raw_stderr, raw_wait_thr = Open3.popen3( + env.merge({ "COLUMNS" => Tty.width.to_s }), + [executable, executable], + *args, + **options, + ) write_input_to(raw_stdin) raw_stdin.close_write From b8847a90d98a638fd907739eb3486b89bd491651 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sun, 14 Jul 2024 15:08:37 -0400 Subject: [PATCH 9/9] Remove unused argument. --- Library/Homebrew/extend/kernel.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/Library/Homebrew/extend/kernel.rb b/Library/Homebrew/extend/kernel.rb index 79f566b0f0..61d1e0b610 100644 --- a/Library/Homebrew/extend/kernel.rb +++ b/Library/Homebrew/extend/kernel.rb @@ -351,16 +351,14 @@ module Kernel IGNORE_INTERRUPTS_MUTEX = Thread::Mutex.new.freeze - def ignore_interrupts(quiet: false) + def ignore_interrupts IGNORE_INTERRUPTS_MUTEX.synchronize do interrupted = T.let(false, T::Boolean) old_sigint_handler = trap(:INT) do interrupted = true - unless quiet - $stderr.print "\n" - $stderr.puts "One sec, cleaning up..." - end + $stderr.print "\n" + $stderr.puts "One sec, cleaning up..." end begin