[Bug #22415] Restore ec->errinfo after handling interrupts

Interrupts are checked at arbitrary points, including while a jump
(break/next/redo/return/raise/throw) is being propagated, and the payload
of that jump lives in ec->errinfo.

Handling an interrupt can run Ruby code: rb_signal_exec() runs a trap
handler and threadptr_interrupt_exec_exec() runs interrupt_exec tasks.
Neither preserves ec->errinfo, and any rescue or ensure clause that code
enters sets it to nil.

Thread.handle_interrupt checks for interrupts after its block returns but
before re-raising the jump that left the block.  If a signal handler runs
in that window the throw data is replaced with nil and the VM dereferences
nil as a vm_throw_data:

    THROW_DATA_CATCH_FRAME (obj=0x4) at vm_insnhelper.h:217
    vm_exec_handle_exception (ec=..., errinfo=4) at vm.c:2880
    vm_exec_loop / rb_vm_exec at vm.c:2829 / vm.c:2812
    invoke_iseq_block_from_c / rb_yield / rb_ary_each

rb_postponed_job_flush() and rb_exec_event_hooks() already save and restore
ec->errinfo around the Ruby code they run; do the same for the rest of
rb_threadptr_execute_interrupts().  The paths that set errinfo on purpose
(rb_exc_raise, rb_threadptr_to_kill) jump out of the function and so do not
reach the restore.

Author: rwstauner

test/ruby/test_thread.rb

@@ -822,6 +822,59 @@ def test_handle_interrupt_with_break
     end
   end
 
+  # Thread.handle_interrupt checks for interrupts after the block returns,
+  # before re-raising the jump (break/return/raise) that left the block.
+  # Running a signal handler there must not clobber ec->errinfo, which still
+  # holds the payload of that jump.
+  def test_handle_interrupt_with_return_and_trap_handler
+    omit "needs fork" unless Process.respond_to?(:fork)
+    omit "SIGUSR1 is not supported" unless Signal.list.key?("USR1")
+
+    assert_separately([], "#{<<~"begin;"}\n#{<<~'end;'}", timeout: 120)
+    begin;
+      trap(:USR1) do
+        begin
+          raise "entering a rescue clause clears errinfo"
+        rescue
+        end
+      end
+
+      def return_from_handle_interrupt(array)
+        array.each do |i|
+          Thread.handle_interrupt(Exception => :immediate) { return i }
+        end
+        nil
+      end
+
+      # Signal from another process: a thread of this process would only get
+      # scheduled a few times a second while the main thread spins.
+      pid = Process.pid
+      killer = fork do
+        begin
+          # Process.ppid changes if the parent crashes.
+          while Process.ppid == pid
+            Process.kill(:USR1, pid)
+            sleep 0.001
+          end
+        rescue Errno::ESRCH
+        end
+        exit!(0)
+      end
+
+      returned = nil
+      begin
+        500_000.times do
+          returned = return_from_handle_interrupt([1, 2, 3])
+          break unless returned == 1
+        end
+      ensure
+        Process.kill(:KILL, killer) rescue Errno::ESRCH
+        Process.waitpid(killer)
+      end
+      assert_equal(1, returned)
+    end;
+  end
+
   def test_handle_interrupt_blocking
     r = nil
     q = Thread::Queue.new

thread.c

@@ -2416,6 +2416,11 @@ rb_threadptr_execute_interrupts(rb_thread_t *th, int blocking_timing)
 
     if (th->ec->raised_flag) return ret;
 
+    /* Interrupts are checked at arbitrary points, including while a jump
+     * (break/next/redo/return/raise/throw) is being propagated.
+     * Save errinfo as the interrupt might run Ruby code that could clear it. */
+    const VALUE saved_errinfo = th->ec->errinfo;
+
     while ((interrupt = threadptr_get_interrupts(th)) != 0) {
         int sig;
         int timer_interrupt;
@@ -2500,6 +2505,9 @@ rb_threadptr_execute_interrupts(rb_thread_t *th, int blocking_timing)
             rb_thread_schedule_limits(limits_us);
         }
     }
+
+    th->ec->errinfo = saved_errinfo;
+
     return ret;
 }