Project

General

Profile

Bug #17331

Let Fiber#raise work with transferring fibers

Added by nevans (Nicholas Evans) about 2 months ago. Updated about 1 month ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
[ruby-core:100925]

Description

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.


Related issues

Related to Ruby master - Feature #17325: Adds Fiber#cancel, which forces a Fiber to break/returnOpenActions

Updated by nevans (Nicholas Evans) about 2 months ago

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. :)

Updated by nevans (Nicholas Evans) about 2 months ago

  • 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.

Updated by nevans (Nicholas Evans) about 2 months ago

  • 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...)".

Updated by nevans (Nicholas Evans) about 2 months ago

  • Description updated (diff)

updated description

Updated by Eregon (Benoit Daloze) about 2 months ago

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

#6

Updated by Eregon (Benoit Daloze) about 2 months ago

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

Updated by ko1 (Koichi Sasada) about 2 months ago

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?

Updated by ko1 (Koichi Sasada) about 2 months ago

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?

Updated by matz (Yukihiro Matsumoto) about 2 months ago

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.

Updated by nevans (Nicholas Evans) about 2 months ago

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.

Updated by ioquatix (Samuel Williams) about 1 month ago

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

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

Updated by Eregon (Benoit Daloze) about 1 month ago

  • 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.

Also available in: Atom PDF