Merge pull request #17722 from reitermarkus/ignore-interrupts

Make `ignore_interrupts` thread-safe.

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
-          cached_location.dirname.mkpath
+        temporary_path.rename(cached_location)
-          temporary_path.rename(cached_location)
-          symlink_location.dirname.mkpath
-        end
       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?

Library/Homebrew/extend/kernel.rb

@@ -349,30 +349,26 @@ module Kernel
     end
   end
 
-  def ignore_interrupts(_opt = nil)
+  IGNORE_INTERRUPTS_MUTEX = Thread::Mutex.new.freeze
-    # 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)
+  def ignore_interrupts
-    old_sigint_handler = trap(:INT) do
+    IGNORE_INTERRUPTS_MUTEX.synchronize do
-      $ignore_interrupts_interrupted = true
+      interrupted = T.let(false, T::Boolean)
-      $stderr.print "\n"
+      old_sigint_handler = trap(:INT) do
-      $stderr.puts "One sec, cleaning up..."
+        interrupted = true
-    end
 
-    begin
+        $stderr.print "\n"
-      yield
+        $stderr.puts "One sec, cleaning up..."
-    ensure
+      end
-      trap(:INT, old_sigint_handler)
 
-      $ignore_interrupts_nesting_level -= 1
+      begin
-      if $ignore_interrupts_nesting_level == 0 && $ignore_interrupts_interrupted
+        yield
-        $ignore_interrupts_interrupted = false
+      ensure
-        raise Interrupt
+        trap(:INT, old_sigint_handler)
+
+        raise Interrupt if interrupted
       end
     end
-    # rubocop:enable Style/GlobalVars
   end
 
   def redirect_stdout(file)

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
+    raw_stdin, raw_stdout, raw_stderr, raw_wait_thr = Open3.popen3(
-      Open3.popen3(
+      env.merge({ "COLUMNS" => Tty.width.to_s }),
-        env.merge({ "COLUMNS" => Tty.width.to_s }),
+      [executable, executable],
-        [executable, executable],
+      *args,
-        *args,
+      **options,
-        **options,
+    )
-      )
-    end
 
     write_input_to(raw_stdin)
     raw_stdin.close_write

Library/Homebrew/utils/fork.rb

@@ -75,11 +75,13 @@ module Utils
           exit!(true)
         end
 
-        ignore_interrupts(:quietly) 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?
+          Process.waitpid(pid) unless socket.nil?
-
+        rescue Interrupt
-          # 130 is the exit status for a process interrupted via Ctrl-C.
+          Process.waitpid(pid)
-          # 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?
         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

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
-        cached_location.dirname.mkpath
+      temporary_path.rename(cached_location)
-        temporary_path.rename(cached_location)
-        symlink_location.dirname.mkpath
-      end
     end
+
+    symlink_location.dirname.mkpath
     FileUtils.ln_s cached_location.relative_path_from(symlink_location.dirname), symlink_location, force: true
   end