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

• Status: Closed
• Assignee: ioquatix (Samuel Williams)
• Target version: -
• Backport: 3.1: REQUIRED, 3.2: WONTFIX, 3.3: DONE

[ioquatix (Samuel Williams)]

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.

[ioquatix (Samuel Williams)]

Proposed change: https://github.com/ruby/ruby/pull/10482

[ioquatix (Samuel Williams)]

• Related to Feature #20102: Introduce `Fiber#resuming?` added

[ioquatix (Samuel Williams)]

• Description updated (diff)

[ioquatix (Samuel Williams)]

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

[ioquatix (Samuel Williams)]

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

[matz (Yukihiro Matsumoto)]

I see no problem in the proposal. I agree.

Matz.

[ioquatix (Samuel Williams)]

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.

[ioquatix (Samuel Williams)]

• 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

Merged in https://github.com/ruby/ruby/commit/6ade36c06b7cef948099b8f5f483763498705d12.

[k0kubun (Takashi Kokubun)]

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

ruby_3_3 5c06e930748ef6bdb4ac4751ba16b7b604da3db0 merged revision(s) 6ade36c06b7cef948099b8f5f483763498705d12.

[nagachika (Tomoyuki Chikanaga)]

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.

[nagachika (Tomoyuki Chikanaga)]

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

ruby_3_2 2f8f17e842666abb05ca522d6072c957fab0e12e merged revision(s) 5d1702e01a36e11b183fe29ce10780a9b1a41cf0.

[nagachika (Tomoyuki Chikanaga)]

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

[nagachika (Tomoyuki Chikanaga)]

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