[GR-13219] JT: Forward signals to sub-processes, so they get notified when sending a signal to jt

PullRequest: truffleruby/519

Author: eregon

tool/jt.rb

@@ -17,7 +17,6 @@
 require 'json'
 require 'timeout'
 require 'yaml'
-require 'open3'
 require 'rbconfig'
 require 'pathname'
 
@@ -53,9 +52,6 @@
 
 require "#{TRUFFLERUBY_DIR}/lib/truffle/truffle/openssl-prefix.rb"
 
-# wait for sub-processes to handle the interrupt
-trap(:INT) {}
-
 MRI_TEST_MODULES = {
     '--no-sulong' => {
         help: 'exclude all tests requiring Sulong',
@@ -81,6 +77,23 @@
     }
 }
 
+SUBPROCESSES = []
+
+# Forward signals to sub-processes, so they get notified when sending a signal to jt
+[:SIGINT, :SIGTERM].each do |signal|
+  trap(signal) do
+    SUBPROCESSES.each do |pid|
+      puts "\nSending #{signal} to process #{pid}"
+      begin
+        Process.kill(signal, pid)
+      rescue Errno::ESRCH
+        # Already killed
+      end
+    end
+    # Keep running jt which will wait for the subprocesses termination
+  end
+end
+
 module Utilities
   private
 
@@ -131,7 +144,7 @@ def find_graal_javacmd_and_options
       options = ['--no-bootclasspath']
     elsif graal_home
       graal_home = File.expand_path(graal_home, TRUFFLERUBY_DIR)
-      output, _ = mx('-v', '-p', graal_home, 'vm', '-version', :err => :out, capture: true)
+      output = mx('-v', '-p', graal_home, 'vm', '-version', capture: true, :err => :out)
       command_line = output.lines.select { |line| line.include? '-version' }
       if command_line.size == 1
         command_line = command_line[0]
@@ -288,25 +301,34 @@ def diff(expected, actual)
     `diff -u #{expected} #{actual}`
   end
 
-  def system_timeout(timeout, *args)
-    begin
-      pid = Process.spawn(*args)
-    rescue SystemCallError
-      return nil
-    end
+  def raw_sh_failed_status
+    `false`
+    $?
+  end
 
-    begin
-      Timeout.timeout timeout do
-        Process.waitpid pid
-        $?.success?
+  def raw_sh_with_timeout(timeout, pid)
+    if !timeout
+      yield
+    else
+      begin
+        Timeout.timeout(timeout) do
+          yield
+        end
+      rescue Timeout::Error
+        Process.kill('TERM', pid)
+        yield # Wait and read the pipe if capture: true
+        :timeout
       end
-    rescue Timeout::Error
-      Process.kill('TERM', pid)
-      Process.waitpid pid
-      nil
     end
   end
 
+  def raw_sh_track_subprocess(pid)
+    SUBPROCESSES << pid
+    yield
+  ensure
+    SUBPROCESSES.delete(pid)
+  end
+
   def raw_sh(*args)
     options = args.last.is_a?(Hash) ? args.last : {}
     continue_on_failure = options.delete :continue_on_failure
@@ -318,44 +340,51 @@ def raw_sh(*args)
       STDERR.puts "$ #{printable_cmd(args)}"
     end
 
-    if use_exec
-      result = exec(*args)
-    elsif timeout
-      result = system_timeout(timeout, *args)
-    elsif capture
-      if options.delete(:out) == :err
-        err, status = Open3.capture2e(*args)
-        out = ""
-      else
-        out, err, status = Open3.capture3(*args)
-      end
-      result = status.success?
-    else
-      result = system(*args)
+    exec(*args) if use_exec
+
+    if capture
+      raise ":capture can only be combined with :err => :out" if options.include?(:out)
+      pipe_r, pipe_w = IO.pipe
+      options[:out] = pipe_w
+      options[:err] = pipe_w if options[:err] == :out
     end
 
-    if result
-      if capture
-        [out, err]
-      else
-        true
-      end
-    elsif continue_on_failure
-      false
+    status = nil
+    out = nil
+    begin
+      pid = Process.spawn(*args)
+    rescue Errno::ENOENT # No such executable
+      status = raw_sh_failed_status
     else
-      status = $? unless capture
-      $stderr.puts "FAILED (#{status}): #{printable_cmd(args)}"
+      raw_sh_track_subprocess(pid) do
+        pipe_w.close if capture
 
-      if capture
-        $stderr.puts out
-        $stderr.puts err
+        result = raw_sh_with_timeout(timeout, pid) do
+          out = pipe_r.read if capture
+          _, status = Process.waitpid2(pid)
+        end
+        if result == :timeout
+          status = raw_sh_failed_status
+        end
       end
+    end
 
-      if status && status.exitstatus
-        exit status.exitstatus
+    result = status.success?
+
+    if capture
+      pipe_r.close
+    end
+
+    if status.success? || continue_on_failure
+      if capture
+        out
       else
-        exit 1
+        status.success?
       end
+    else
+      $stderr.puts "FAILED (#{status}): #{printable_cmd(args)}"
+      $stderr.puts out if capture
+      exit status.to_i
     end
   end
 
@@ -794,7 +823,7 @@ def pr_clean(*args)
       puts "Open PRs: #{open_prs}"
 
       sh 'git', 'fetch', Remotes.bitbucket, '--prune' # ensure we have locally only existing remote branches
-      branches, _        = sh 'git', 'branch', '--remote', '--list', capture: true
+      branches = sh 'git', 'branch', '--remote', '--list', capture: true
       branches_to_delete = branches.
           scan(/^ *#{Remotes.bitbucket}\/(github\/pr\/(\d+))$/).
           reject { |_, number| open_prs.include? Integer(number) }
@@ -814,7 +843,7 @@ def pr_clean(*args)
     def pr_push(*args)
       # Fetch PRs on GitHub
       fetch     = "+refs/pull/*/head:refs/remotes/#{Remotes.github}/pr/*"
-      out, _err = sh 'git', 'config', '--get-all', "remote.#{Remotes.github}.fetch", capture: true
+      out = sh 'git', 'config', '--get-all', "remote.#{Remotes.github}.fetch", capture: true
       sh 'git', 'config', '--add', "remote.#{Remotes.github}.fetch", fetch unless out.include? fetch
       sh 'git', 'fetch', Remotes.github
 
@@ -823,7 +852,7 @@ def pr_push(*args)
         github_pr_branch = "#{Remotes.github}/pr/#{pr_number}"
       else
         github_pr_branch = begin
-          out, _err = sh 'git', 'branch', '-r', '--contains', 'HEAD', capture: true
+          out = sh 'git', 'branch', '-r', '--contains', 'HEAD', capture: true
           candidate = out.lines.find { |l| l.strip.start_with? "#{Remotes.github}/pr/" }
           candidate && candidate.strip.chomp
         end
@@ -868,9 +897,9 @@ def github(dir = TRUFFLERUBY_DIR)
     def remote_urls(dir = TRUFFLERUBY_DIR)
       @remote_urls ||= Hash.new
       @remote_urls[dir] ||= begin
-        out, _err = raw_sh 'git', '-C', dir, 'remote', capture: true, no_print_cmd: true
+        out = raw_sh 'git', '-C', dir, 'remote', capture: true, no_print_cmd: true
         out.split.map do |remote|
-          url, _err = raw_sh 'git', '-C', dir, 'config', '--get', "remote.#{remote}.url", capture: true, no_print_cmd: true
+          url = raw_sh 'git', '-C', dir, 'config', '--get', "remote.#{remote}.url", capture: true, no_print_cmd: true
           [remote, url.chomp]
         end
       end
@@ -1112,7 +1141,7 @@ def test_cexts(*args)
 
         sh 'clang', '-c', '-emit-llvm', *openssl_cflags, 'test/truffle/cexts/xopenssl/main.c', '-o', 'test/truffle/cexts/xopenssl/main.bc'
         mx 'build', '--dependencies', 'SULONG_LAUNCHER' # For mx lli
-        out, _ = mx('lli', "-Dpolyglot.llvm.libraries=#{openssl_lib}", 'test/truffle/cexts/xopenssl/main.bc', capture: true)
+        out = mx('lli', "-Dpolyglot.llvm.libraries=#{openssl_lib}", 'test/truffle/cexts/xopenssl/main.bc', capture: true)
         raise out.inspect unless out == "5d41402abc4b2a76b9719d911017c592\n"
 
       when 'minimum', 'method', 'module', 'globals', 'backtraces', 'xopenssl'
@@ -1458,7 +1487,6 @@ def build_stats_native_runtime_compilable_methods(*args)
   end
 
   def metrics(command, *args)
-    trap(:INT) { puts; exit }
     args = args.dup
     case command
     when 'alloc'
@@ -1481,8 +1509,8 @@ def metrics_alloc(*args)
     samples = []
     METRICS_REPS.times do
       log '.', "sampling\n"
-      out, err = run_ruby '-J-Dtruffleruby.metrics.memory_used_on_exit=true', '-J-verbose:gc', *args, capture: true, no_print_cmd: true
-      samples.push memory_allocated(out+err)
+      out = run_ruby '-J-Dtruffleruby.metrics.memory_used_on_exit=true', '-J-verbose:gc', *args, capture: true, :err => :out, no_print_cmd: true
+      samples.push memory_allocated(out)
     end
     log "\n", nil
     range = samples.max - samples.min
@@ -1568,12 +1596,12 @@ def metrics_maxrss(*args)
       log '.', "sampling\n"
 
       max_rss_in_mb = if LINUX
-                        out, err = raw_sh('/usr/bin/time', '-v', '--', find_launcher(true), *args, capture: true, no_print_cmd: true)
-                        err =~ /Maximum resident set size \(kbytes\): (?<max_rss_in_kb>\d+)/m
+                        out = raw_sh('/usr/bin/time', '-v', '--', find_launcher(true), *args, capture: true, :err => :out, no_print_cmd: true)
+                        out =~ /Maximum resident set size \(kbytes\): (?<max_rss_in_kb>\d+)/m
                         Integer($~[:max_rss_in_kb]) / 1024.0
                       elsif MAC
-                        out, err = raw_sh('/usr/bin/time', '-l', '--', find_launcher(true), *args, capture: true, no_print_cmd: true)
-                        err =~ /(?<max_rss_in_bytes>\d+)\s+maximum resident set size/m
+                        out = raw_sh('/usr/bin/time', '-l', '--', find_launcher(true), *args, capture: true, :err => :out, no_print_cmd: true)
+                        out =~ /(?<max_rss_in_bytes>\d+)\s+maximum resident set size/m
                         Integer($~[:max_rss_in_bytes]) / 1024.0 / 1024.0
                       else
                         raise "Can't measure RSS on this platform."
@@ -1610,9 +1638,9 @@ def metrics_native_instructions(*args)
 
     use_json = args.delete '--json'
 
-    out, err = raw_sh('perf', 'stat', '-e', 'instructions', '--', find_launcher(true), *args, capture: true, no_print_cmd: true)
+    out = raw_sh('perf', 'stat', '-e', 'instructions', '--', find_launcher(true), *args, capture: true, :err => :out, no_print_cmd: true)
 
-    err =~ /(?<instruction_count>[\d,]+)\s+instructions/m
+    out =~ /(?<instruction_count>[\d,]+)\s+instructions/m
     instruction_count = $~[:instruction_count].gsub(',', '')
 
     log "\n", nil
@@ -1639,9 +1667,9 @@ def metrics_time(*args)
     samples = METRICS_REPS.times.map do
       log '.', "sampling\n"
       start = Time.now
-      _, err = run_ruby(*args, capture: true, no_print_cmd: true, :out => :err)
+      out = run_ruby(*args, capture: true, no_print_cmd: true, :err => :out)
       finish = Time.now
-      get_times(err, (finish - start) * 1000.0)
+      get_times(out, (finish - start) * 1000.0)
     end
     log "\n", nil
 
@@ -1780,22 +1808,19 @@ def profile(*args)
     run_args = *DEFAULT_PROFILE_OPTIONS + args
 
     begin
-      profile_data, err = run_ruby(env, *run_args, capture: true)
-      $stderr.puts(err) unless err.empty?
+      profile_data = run_ruby(env, *run_args, capture: true)
 
       profile_data_file = Tempfile.new %w[truffleruby-profile .json]
       profile_data_file.write(profile_data)
       profile_data_file.close
 
-      flamegraph_data, err = raw_sh "#{repo}/stackcollapse-graalvm.rb", profile_data_file.path, capture: true
-      $stderr.puts(err) unless err.empty?
+      flamegraph_data = raw_sh "#{repo}/stackcollapse-graalvm.rb", profile_data_file.path, capture: true
 
       flamegraph_data_file = Tempfile.new 'truffleruby-flamegraph-data'
       flamegraph_data_file.write(flamegraph_data)
       flamegraph_data_file.close
 
-      svg_data, err = raw_sh "#{repo}/flamegraph.pl", flamegraph_data_file.path, capture: true
-      $stderr.puts(err) unless err.empty?
+      svg_data = raw_sh "#{repo}/flamegraph.pl", flamegraph_data_file.path, capture: true
 
       Dir.mkdir(PROFILES_DIR) unless Dir.exist?(PROFILES_DIR)
       svg_filename = "#{PROFILES_DIR}/flamegraph_#{Time.now.strftime("%Y%m%d-%H%M%S")}.svg"
@@ -1954,7 +1979,7 @@ def check_filename_length
 
   def check_parser
     build('parser')
-    diff, _err = sh 'git', 'diff', 'src/main/java/org/truffleruby/parser/parser/RubyParser.java', :err => :out, capture: true
+    diff = sh 'git', 'diff', 'src/main/java/org/truffleruby/parser/parser/RubyParser.java', capture: true
     unless diff.empty?
       STDERR.puts "DIFF:"
       STDERR.puts diff