Send :fiber_switch event for almost every fiber_switch (#4207)

With this patch, TracePoint receives a `:fiber_switch` event for
_almost_ every fiber switch.  Previously, it would not be sent when an
exception was going to be raised. Now the event should only be blockable
by an interrupt (including `Thread#raise`) or a fatal error.

Additionally, interrupts will now be checked on the return fiber
_before_ re-raising the terminating unhandled exception.  And a fiber
that terminates with an unhandled exception no longer creates a pending
interrupt on its thread.  The exception will be raised in the return
fiber the same way as `Fiber#raise`: using `cont.value` with `cont.argc
== -1`

I moved `rb_exc_raise` from `fiber_store` to the end of `fiber_switch`
after _all_ of the other cleanup code: `fiber_stack_release`,
`th->blocking` increment, `RUBY_VM_CHECK_INTS`, and `EXEC_EVENT_HOOK`.
It seems to me that skipping those other cleanup steps may have also
resulted in other bugs.

cont.c

@@ -2005,7 +2005,7 @@ rb_fiber_set_scheduler(VALUE klass, VALUE scheduler)
     return rb_fiber_scheduler_set(scheduler);
 }
 
-static void rb_fiber_terminate(rb_fiber_t *fiber, int need_interrupt);
+static void rb_fiber_terminate(rb_fiber_t *fiber, int need_interrupt, VALUE err);
 
 void
 rb_fiber_start(void)
@@ -2015,6 +2015,7 @@ rb_fiber_start(void)
     rb_proc_t *proc;
     enum ruby_tag_type state;
     int need_interrupt = TRUE;
+    VALUE err = Qfalse;
 
     VM_ASSERT(th->ec == GET_EC());
     VM_ASSERT(FIBER_RESUMED_P(fiber));
@@ -2041,22 +2042,21 @@ rb_fiber_start(void)
     EC_POP_TAG();
 
     if (state) {
-        VALUE err = th->ec->errinfo;
+        err = th->ec->errinfo;
         VM_ASSERT(FIBER_RESUMED_P(fiber));
 
-        if (state == TAG_RAISE || state == TAG_FATAL) {
+        if (state == TAG_RAISE) {
+            // noop...
+        } else if (state == TAG_FATAL) {
             rb_threadptr_pending_interrupt_enque(th, err);
         }
         else {
             err = rb_vm_make_jump_tag_but_local_jump(state, err);
-            if (!NIL_P(err)) {
-                rb_threadptr_pending_interrupt_enque(th, err);
-            }
         }
         need_interrupt = TRUE;
     }
 
-    rb_fiber_terminate(fiber, need_interrupt);
+    rb_fiber_terminate(fiber, need_interrupt, err);
     VM_UNREACHABLE(rb_fiber_start);
 }
 
@@ -2182,7 +2182,7 @@ rb_fiber_current(void)
 }
 
 // Prepare to execute next_fiber on the given thread.
-static inline VALUE
+static inline void
 fiber_store(rb_fiber_t *next_fiber, rb_thread_t *th)
 {
     rb_fiber_t *fiber;
@@ -2206,13 +2206,6 @@ fiber_store(rb_fiber_t *next_fiber, rb_thread_t *th)
 
     fiber_status_set(next_fiber, FIBER_RESUMED);
     fiber_setcontext(next_fiber, fiber);
-
-    fiber = th->ec->fiber_ptr;
-
-    /* Raise an exception if that was the result of executing the fiber */
-    if (fiber->cont.argc == -1) rb_exc_raise(fiber->cont.value);
-
-    return fiber->cont.value;
 }
 
 static inline VALUE
@@ -2285,7 +2278,7 @@ fiber_switch(rb_fiber_t *fiber, int argc, const VALUE *argv, int kw_splat, VALUE
     cont->kw_splat = kw_splat;
     cont->value = make_passing_arg(argc, argv);
 
-    value = fiber_store(fiber, th);
+    fiber_store(fiber, th);
 
     if (RTEST(resuming_fiber) && FIBER_TERMINATED_P(fiber)) {
         fiber_stack_release(fiber);
@@ -2299,6 +2292,9 @@ fiber_switch(rb_fiber_t *fiber, int argc, const VALUE *argv, int kw_splat, VALUE
 
     EXEC_EVENT_HOOK(th->ec, RUBY_EVENT_FIBER_SWITCH, th->self, 0, 0, 0, Qnil);
 
+    current_fiber = th->ec->fiber_ptr;
+    value = current_fiber->cont.value;
+    if (current_fiber->cont.argc == -1) rb_exc_raise(value);
     return value;
 }
 
@@ -2365,7 +2361,7 @@ rb_fiber_close(rb_fiber_t *fiber)
 }
 
 static void
-rb_fiber_terminate(rb_fiber_t *fiber, int need_interrupt)
+rb_fiber_terminate(rb_fiber_t *fiber, int need_interrupt, VALUE err)
 {
     VALUE value = fiber->cont.value;
     rb_fiber_t *next_fiber;
@@ -2380,7 +2376,11 @@ rb_fiber_terminate(rb_fiber_t *fiber, int need_interrupt)
 
     next_fiber = return_fiber(true);
     if (need_interrupt) RUBY_VM_SET_INTERRUPT(&next_fiber->cont.saved_ec);
-    fiber_switch(next_fiber, 1, &value, RB_NO_KEYWORDS, Qfalse, false);
+    if (RTEST(err))
+        fiber_switch(next_fiber, -1, &err, RB_NO_KEYWORDS, Qfalse, false);
+    else
+        fiber_switch(next_fiber, 1, &value, RB_NO_KEYWORDS, Qfalse, false);
+    VM_UNREACHABLE(rb_fiber_terminate);
 }
 
 VALUE

test/ruby/test_settracefunc.rb

@@ -1593,6 +1593,33 @@ class TestSetTraceFunc < Test::Unit::TestCase
       assert_equal ev, :fiber_switch
     }
 
+    # test for raise into resumable fiber
+    evs = []
+    f = nil
+    TracePoint.new(:raise, :fiber_switch){|tp|
+      next unless target_thread?
+      evs << [tp.event, Fiber.current]
+    }.enable{
+      f = Fiber.new{
+        Fiber.yield # will raise
+        Fiber.yield # unreachable
+      }
+      begin
+        f.resume
+        f.raise StopIteration
+      rescue StopIteration
+        evs << :rescued
+      end
+    }
+    assert_equal [:fiber_switch, f],             evs[0], "initial resume"
+    assert_equal [:fiber_switch, Fiber.current], evs[1], "Fiber.yield"
+    assert_equal [:fiber_switch, f],             evs[2], "fiber.raise"
+    assert_equal [:raise, f],                    evs[3], "fiber.raise"
+    assert_equal [:fiber_switch, Fiber.current], evs[4], "terminated with raise"
+    assert_equal [:raise, Fiber.current],        evs[5], "terminated with raise"
+    assert_equal :rescued,                       evs[6]
+    assert_equal 7, evs.size
+
     # test for transfer
     evs = []
     TracePoint.new(:fiber_switch){|tp|
@@ -1615,6 +1642,41 @@ class TestSetTraceFunc < Test::Unit::TestCase
     evs.each{|ev|
       assert_equal ev, :fiber_switch
     }
+
+    # test for raise and from transferring fibers
+    evs = []
+    f1 = f2 = nil
+    TracePoint.new(:raise, :fiber_switch){|tp|
+      next unless target_thread?
+      evs << [tp.event, Fiber.current]
+    }.enable{
+      f1 = Fiber.new{
+        f2.transfer
+        f2.raise ScriptError
+        Fiber.yield :ok
+      }
+      f2 = Fiber.new{
+        f1.transfer
+        f1.transfer
+      }
+      begin
+        f1.resume
+      rescue ScriptError
+        evs << :rescued
+      end
+    }
+    assert_equal [:fiber_switch, f1],            evs[0], "initial resume"
+    assert_equal [:fiber_switch, f2],            evs[1], "f2.transfer"
+    assert_equal [:fiber_switch, f1],            evs[2], "f1.transfer"
+    assert_equal [:fiber_switch, f2],            evs[3], "f2.raise ScriptError"
+    assert_equal [:raise,        f2],            evs[4], "f2.raise ScriptError"
+    assert_equal [:fiber_switch, f1],            evs[5], "f2 unhandled exception"
+    assert_equal [:raise,        f1],            evs[6], "f2 unhandled exception"
+    assert_equal [:fiber_switch, Fiber.current], evs[7], "f1 unhandled exception"
+    assert_equal [:raise,        Fiber.current], evs[8], "f1 unhandled exception"
+    assert_equal :rescued,                       evs[9], "rescued everything"
+    assert_equal 10, evs.size
+
   end
 
   def test_tracepoint_callee_id