ruby-changes:65313
From: nicholas <ko1@a...>
Date: Mon, 22 Feb 2021 06:33:31 +0900 (JST)
Subject: [ruby-changes:65313] 3ee4fa9491 (master): Send :fiber_switch event for almost every fiber_switch (#4207)
https://git.ruby-lang.org/ruby.git/commit/?id=3ee4fa9491 From 3ee4fa9491d0b2b5fb40deea8e93e797924de789 Mon Sep 17 00:00:00 2001 From: "nicholas a. evans" <nicholas.evans@g...> Date: Sun, 21 Feb 2021 16:33:11 -0500 Subject: 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 | 36 ++++++++++++------------ test/ruby/test_settracefunc.rb | 62 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 18 deletions(-) diff --git a/cont.c b/cont.c index abd6a2f..72ebf6f 100644 --- a/cont.c +++ b/cont.c @@ -2005,7 +2005,7 @@ rb_fiber_set_scheduler(VALUE klass, VALUE scheduler) https://github.com/ruby/ruby/blob/trunk/cont.c#L2005 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) https://github.com/ruby/ruby/blob/trunk/cont.c#L2015 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) https://github.com/ruby/ruby/blob/trunk/cont.c#L2042 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) https://github.com/ruby/ruby/blob/trunk/cont.c#L2182 } // 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) https://github.com/ruby/ruby/blob/trunk/cont.c#L2206 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 https://github.com/ruby/ruby/blob/trunk/cont.c#L2278 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 https://github.com/ruby/ruby/blob/trunk/cont.c#L2292 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) https://github.com/ruby/ruby/blob/trunk/cont.c#L2361 } 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) https://github.com/ruby/ruby/blob/trunk/cont.c#L2376 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 diff --git a/test/ruby/test_settracefunc.rb b/test/ruby/test_settracefunc.rb index 1d16b2e..fdd5bd2 100644 --- a/test/ruby/test_settracefunc.rb +++ b/test/ruby/test_settracefunc.rb @@ -1593,6 +1593,33 @@ class TestSetTraceFunc < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_settracefunc.rb#L1593 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 https://github.com/ruby/ruby/blob/trunk/test/ruby/test_settracefunc.rb#L1642 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 -- cgit v1.1 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/