Project

General

Profile

Actions

Bug #20414

closed

`Fiber#raise` should recurse to `resumed_fiber` rather than failing.

Bug #20414: `Fiber#raise` should recurse to `resumed_fiber` rather than failing.

Added by ioquatix (Samuel Williams) over 1 year ago. Updated over 1 year ago.

Status:
Closed
Target version:
-
[ruby-core:117458]

Description

The following program will fail with FiberError, and is difficult to properly clean up:

root_fiber = Fiber.current f1 = Fiber.new do root_fiber.transfer end f2 = Fiber.new do f1.resume end f2.transfer f2.raise("error") # => `raise': attempt to transfer to a resuming fiber (FiberError) 

This program deliberately set's up a scenario where f2 is resuming f1. Trying to raise an exception on f2 is impossible, because the only way control flow can return to it, is when f1 yields or exits.

We can avoid this problem, by raising the exception on f1, and we can do this automatically using the following logic:

static VALUE fiber_raise(rb_fiber_t *fiber, VALUE exception) { // Add this recursive step: if (fiber->resuming_fiber) { return fiber_raise(fiber->resuming_fiber, exception); } // Existing code ... else if (FIBER_SUSPENDED_P(fiber) && !fiber->yielding) { return fiber_transfer_kw(fiber, -1, &exception, RB_NO_KEYWORDS); } else { return fiber_resume_kw(fiber, -1, &exception, RB_NO_KEYWORDS); } } 

This makes Fiber#raise much more robust and useful for the purpose of stopping fibers, without knowing exactly what they are doing.


Related issues 2 (1 open1 closed)

Updated by ioquatix (Samuel Williams) over 1 year ago Actions #2

Updated by ioquatix (Samuel Williams) over 1 year ago Actions #3

  • Description updated (diff)

Updated by ioquatix (Samuel Williams) over 1 year ago Actions #4 [ruby-core:117461]

With the proposed change, the following program:

root_fiber = Fiber.current f1 = Fiber.new do puts "f1 enter" root_fiber.transfer puts "f1 exit" ensure puts "f1 ensure" end f2 = Fiber.new do puts "f2 enter" f1.resume puts "f2 exit" ensure puts "f2 ensure" end f2.transfer begin puts "raise" f2.raise(Interrupt) rescue Interrupt => e puts "rescue Interrupt" end 

... generates the following output:

f2 enter f1 enter raise f1 ensure f2 ensure rescue Interrupt 

Updated by ioquatix (Samuel Williams) over 1 year ago Actions #5

  • Related to Bug #20089: Fiber#kill transfers to root fiber added

Updated by matz (Yukihiro Matsumoto) over 1 year ago Actions #6 [ruby-core:117548]

I see no problem in the proposal. I agree.

Matz.

Updated by ioquatix (Samuel Williams) over 1 year ago Actions #7 [ruby-core:117555]

Thanks @matz (Yukihiro Matsumoto).

To clarify, there are two issues are we addressing.

(1) Improve consistency of Thread.current.raise and Fiber.current.raise.
(2) Follow resuming_fiber when raising exceptions.

They overlap, so are addressed by the same PR.

Updated by ioquatix (Samuel Williams) over 1 year ago Actions #8 [ruby-core:117563]

  • Status changed from Open to Closed
  • Backport changed from 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN to 3.1: REQUIRED, 3.2: REQUIRED, 3.3: REQUIRED

Updated by k0kubun (Takashi Kokubun) over 1 year ago Actions #9 [ruby-core:118076]

  • Backport changed from 3.1: REQUIRED, 3.2: REQUIRED, 3.3: REQUIRED to 3.1: REQUIRED, 3.2: REQUIRED, 3.3: DONE

Updated by nagachika (Tomoyuki Chikanaga) over 1 year ago Actions #10 [ruby-core:118453]

I think this change seems somewhat like a spec change. Could this change potentially reveal latent errors in the application and affect its operation?

Additionally, regardless of the case, I believe the ruby_version_is guard in rubyspec will need to be updated if we backport it to stable branches.

Updated by nagachika (Tomoyuki Chikanaga) over 1 year ago Actions #11 [ruby-core:118454]

  • Backport changed from 3.1: REQUIRED, 3.2: REQUIRED, 3.3: DONE to 3.1: REQUIRED, 3.2: DONE, 3.3: DONE

Updated by nagachika (Tomoyuki Chikanaga) over 1 year ago Actions #12 [ruby-core:118455]

  • Backport changed from 3.1: REQUIRED, 3.2: DONE, 3.3: DONE to 3.1: REQUIRED, 3.2: REQUIRED, 3.3: DONE

Sorry, I accidentally changed a different ticket.

Updated by nagachika (Tomoyuki Chikanaga) over 1 year ago Actions #13

  • Backport changed from 3.1: REQUIRED, 3.2: REQUIRED, 3.3: DONE to 3.1: REQUIRED, 3.2: WONTFIX, 3.3: DONE
Actions

Also available in: PDF Atom