https://bugs.ruby-lang.org/https://bugs.ruby-lang.org/favicon.ico?17113305112020-11-18T01:04:07ZRuby Issue Tracking SystemRuby master - Bug #17331: Let Fiber#raise work with transferring fibershttps://bugs.ruby-lang.org/issues/17331?journal_id=885762020-11-18T01:04:07Znevans (Nicholas Evans)
<ul></ul><p>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. :)</p> Ruby master - Bug #17331: Let Fiber#raise work with transferring fibershttps://bugs.ruby-lang.org/issues/17331?journal_id=886422020-11-20T14:27:45Znevans (Nicholas Evans)
<ul><li><strong>Description</strong> updated (<a title="View differences" href="/journals/88642/diff?detail_id=58342">diff</a>)</li></ul><p>I added an alternate implicit implementation at Samuel Williams's request: <a href="https://github.com/ruby/ruby/pull/3795" class="external">https://github.com/ruby/ruby/pull/3795</a></p>
<p>It will automatically choose between <code>rb_fiber_transfer_kw</code> for transferring fibers and <code>rb_fiber_resume_kw</code> for yielding fibers. It will not work on resuming fibers. I prefer the explicit version.</p> Ruby master - Bug #17331: Let Fiber#raise work with transferring fibershttps://bugs.ruby-lang.org/issues/17331?journal_id=886512020-11-20T17:36:04Znevans (Nicholas Evans)
<ul><li><strong>Description</strong> updated (<a title="View differences" href="/journals/88651/diff?detail_id=58348">diff</a>)</li></ul><p>updated description with more details .</p>
<p>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 <code>Fiber.yield</code>). 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...)".</p> Ruby master - Bug #17331: Let Fiber#raise work with transferring fibershttps://bugs.ruby-lang.org/issues/17331?journal_id=886522020-11-20T17:43:39Znevans (Nicholas Evans)
<ul><li><strong>Description</strong> updated (<a title="View differences" href="/journals/88652/diff?detail_id=58349">diff</a>)</li></ul><p>updated description</p> Ruby master - Bug #17331: Let Fiber#raise work with transferring fibershttps://bugs.ruby-lang.org/issues/17331?journal_id=886552020-11-20T19:08:07ZEregon (Benoit Daloze)
<ul></ul><p>This new alternative sounds good to me. I also commented on the PR.</p> Ruby master - Bug #17331: Let Fiber#raise work with transferring fibershttps://bugs.ruby-lang.org/issues/17331?journal_id=886842020-11-22T11:04:07ZEregon (Benoit Daloze)
<ul><li><strong>Related to</strong> <i><a class="issue tracker-2 status-5 priority-4 priority-default closed" href="/issues/17325">Feature #17325</a>: Adds Fiber#cancel, which forces a Fiber to break/return</i> added</li></ul> Ruby master - Bug #17331: Let Fiber#raise work with transferring fibershttps://bugs.ruby-lang.org/issues/17331?journal_id=887052020-11-24T02:32:43Zko1 (Koichi Sasada)
<ul></ul><p>Sorry, could you explain the motivation and specification?<br>
It seems there are many discussion on GH and other tickets, quoting them is great if already well described.</p>
<p>In general, exception on the transferred fibers will return to root fibers. Do you expect it?</p> Ruby master - Bug #17331: Let Fiber#raise work with transferring fibershttps://bugs.ruby-lang.org/issues/17331?journal_id=887382020-11-25T01:32:07Zko1 (Koichi Sasada)
<ul></ul><p>I heard from Samuel, it is used to raise an timeout error or something on a fiber scheduler.<br>
Because the fiber scheduler manages fibers by transferring and it is needed.</p>
<hr>
<p>I'm not sure what is implicit approach and what is explicit approach. Could you summarize the spec?</p> Ruby master - Bug #17331: Let Fiber#raise work with transferring fibershttps://bugs.ruby-lang.org/issues/17331?journal_id=887522020-11-26T05:08:36Zmatz (Yukihiro Matsumoto)matz@ruby.or.jp
<ul></ul><p>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.</p>
<p>Matz.</p> Ruby master - Bug #17331: Let Fiber#raise work with transferring fibershttps://bugs.ruby-lang.org/issues/17331?journal_id=887832020-11-26T20:42:08Znevans (Nicholas Evans)
<ul></ul><p>Thanks for taking a look.</p>
<p>ko1 (Koichi Sasada) wrote in <a href="#note-7">#note-7</a>:</p>
<blockquote>
<p>Sorry, could you explain the motivation and specification?<br>
It seems there are many discussion on GH and other tickets, quoting them is great if already well described.</p>
</blockquote>
<p>Motivation: ruby 2.7 <code>Fiber#raise</code> only works with yielding fibers (although this limitation isn't documented). <code>Fiber#raise</code> should be usable with transferring fibers as well.</p>
<p>So, with ruby 2.7:</p>
<pre><code>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>
</code></pre>
<p>With this patch:</p>
<pre><code>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>
</code></pre>
<blockquote>
<p>In general, exception on the transferred fibers will return to root fibers. Do you expect it?</p>
</blockquote>
<p>Yes. Because transferred fibers always return to root, calling <code>Fiber#raise</code> should be coordinated with or called by a scheduler.</p>
<blockquote>
<p>I'm not sure what is implicit approach and what is explicit approach. Could you summarize the spec?</p>
</blockquote>
<p>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 <code>transfer: true</code> keyword arg when using it with transferring fibers. But Samuel convinced me that the implicit version is fine.</p> Ruby master - Bug #17331: Let Fiber#raise work with transferring fibershttps://bugs.ruby-lang.org/issues/17331?journal_id=889322020-12-06T02:02:16Zioquatix (Samuel Williams)samuel@oriontransfer.net
<ul><li><strong>Tracker</strong> changed from <i>Feature</i> to <i>Bug</i></li><li><strong>Backport</strong> set to <i>2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN</i></li></ul><p>I consider this a bug fix rather than an interface change. I'll review it and consider merging it today.</p> Ruby master - Bug #17331: Let Fiber#raise work with transferring fibershttps://bugs.ruby-lang.org/issues/17331?journal_id=892032020-12-13T12:16:58ZEregon (Benoit Daloze)
<ul><li><strong>Status</strong> changed from <i>Open</i> to <i>Closed</i></li></ul><p><a href="https://github.com/ruby/ruby/pull/3795" class="external">https://github.com/ruby/ruby/pull/3795</a> is merged, closing.</p>
<p>IMHO this is not a bug but additional behavior, I don't think it makes sense to backport.</p>