Bug #17331: Let Fiber#raise work with transferring fibers

• Status: Closed
• Assignee: -
• Target version: -
• Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN

[nevans (Nicholas Evans)]

It would be useful to use raise on transferring fibers just as we can with yielding fibers.

I've added a transfer kwarg, so it is not automatic; the caller must know how to handle the fiber. If you call a yielding fiber with transfer: true or a transferring fiber without transfer: true, a FiberError will be raised. Resuming fibers still raise a FiberError.

yielding_fiber.raise "message"
# => resumes and raises from the last Fiber.yield

transferring_fiber.raise "message", transfer: true
# => transfers and raises from the last fiber.transfer

resuming_fiber.raise "message"
# => raises FiberError

Implementation: https://github.com/ruby/ruby/pull/3783

I also implemented a second version that implicitly and automatically selects rb_fiber_transfer_kw for transferring fibers and rb_fiber_resume_kw for yielding fibers. The implicit version also raises FiberError on resuming fibers.

yielding_fiber.raise "message"
# => resumes and raises from the last Fiber.yield

transferring_fiber.raise "message"
# => transfers and raises from the last fiber.transfer

resuming_fiber.raise "message"
# => raises FiberError

Alternate implicit implementation: https://github.com/ruby/ruby/pull/3795

I slightly prefer the explicit version, but I'm okay with the implicit version.

[nevans (Nicholas Evans)]

We could also make this automatically determine whether the fiber is yielding or transferring and choose the appropriate fiber_switch approach. But transferring vs yielding makes a difference in where control is passed back to when the fiber exits. And this has a high likelihood of causing an immediate follow-up fiber switch (due to termination) back to--where??? return fiber??? last fiber in the root fiber resuming stack???--it feels like it makes at least as much sense to use an explicit arg here as it does to use different methods for resume and transfer. But I don't really mind making it implicit and automatic. :)

[nevans (Nicholas Evans)]

• Description updated (diff)

I added an alternate implicit implementation at Samuel Williams's request: https://github.com/ruby/ruby/pull/3795

It will automatically choose between rb_fiber_transfer_kw for transferring fibers and rb_fiber_resume_kw for yielding fibers. It will not work on resuming fibers. I prefer the explicit version.

[nevans (Nicholas Evans)]

• Description updated (diff)

updated description with more details .

I don't know whether the API should be explicit or implicit. I slightly favor the explicit version, because (in my mental model) resume vs transfer (generator vs full coroutine) are more different from each other than resume and raise (which are merely two resume values for a fiber, two different results for Fiber.yield). But those distinctions can definitely be blurred. ioquatrix made the good point that "There is no point adding an option for which only introduces failure cases (unless that has value...)".

[nevans (Nicholas Evans)]

• Description updated (diff)

updated description

[Eregon (Benoit Daloze)]

This new alternative sounds good to me. I also commented on the PR.

[Eregon (Benoit Daloze)]

• Related to Feature #17325: Adds Fiber#cancel, which forces a Fiber to break/return added

[ko1 (Koichi Sasada)]

Sorry, could you explain the motivation and specification?
It seems there are many discussion on GH and other tickets, quoting them is great if already well described.

In general, exception on the transferred fibers will return to root fibers. Do you expect it?

[ko1 (Koichi Sasada)]

I heard from Samuel, it is used to raise an timeout error or something on a fiber scheduler.
Because the fiber scheduler manages fibers by transferring and it is needed.

---

I'm not sure what is implicit approach and what is explicit approach. Could you summarize the spec?

[matz (Yukihiro Matsumoto)]

Basically positive for implicit one. The transferred fibers and suspended fibers behave differently, especially in exception propagation. You have to summarize the difference in the document.

Matz.

[nevans (Nicholas Evans)]

Thanks for taking a look.

ko1 (Koichi Sasada) wrote in #note-7:

Sorry, could you explain the motivation and specification?
It seems there are many discussion on GH and other tickets, quoting them is great if already well described.

Motivation: ruby 2.7 Fiber#raise only works with yielding fibers (although this limitation isn't documented). Fiber#raise should be usable with transferring fibers as well.

So, with ruby 2.7:

ruby -rfiber -e 'puts RUBY_VERSION; root = Fiber.current; f1 = Fiber.new { (1..).each do |i| root.transfer(i) end }; 3.times { puts f1.transfer }; p((f1.raise("error now") rescue $!))'
2.7.2
1
2
3
#<FiberError: cannot resume transferred Fiber>

With this patch:

tool/runruby.rb -rfiber -e 'puts RUBY_VERSION; root = Fiber.current; f1 = Fiber.new { (1..).each do |i| root.transfer(i) end }; 3.times { puts f1.transfer }; p((f1.raise("error now") rescue $!))'
3.0.0
1
2
3
#<RuntimeError: error now>

In general, exception on the transferred fibers will return to root fibers. Do you expect it?

Yes. Because transferred fibers always return to root, calling Fiber#raise should be coordinated with or called by a scheduler.

I'm not sure what is implicit approach and what is explicit approach. Could you summarize the spec?

Because transferring fibers might not return control back to the current fiber and might require coordination with a scheduler, I thought it would be safest to require a transfer: true keyword arg when using it with transferring fibers. But Samuel convinced me that the implicit version is fine.

[ioquatix (Samuel Williams)]

• Tracker changed from Feature to Bug
• Backport set to 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN

I consider this a bug fix rather than an interface change. I'll review it and consider merging it today.

[Eregon (Benoit Daloze)]

• Status changed from Open to Closed

https://github.com/ruby/ruby/pull/3795 is merged, closing.

IMHO this is not a bug but additional behavior, I don't think it makes sense to backport.