Project

General

Profile

Actions

Bug #20081

open

Transfered Fiber doesn't return to Fiber that started it

Bug #20081: Transfered Fiber doesn't return to Fiber that started it
1

Added by rmosolgo (Robert Mosolgo) over 2 years ago. Updated 5 days ago.

Status:
Open
Assignee:
-
Target version:
-
ruby -v:
ruby 3.2.2 (2023-03-30 revision e51014f9c0) [x86_64-darwin22]
[ruby-core:115879]

Description

Hi! I'm trying to figure out how to make sure that Fibers started with .transfer end up terminated, not just suspended. (If they're suspended, Rails thinks they're still alive, and they continue to hold onto database connections, see: https://github.com/rmosolgo/graphql-ruby/issues/4739#issuecomment-1866930914.)

So, I'm looking for way to make sure that any Fiber I start with .transfer will be properly terminated. But what I noticed is that when a transfer-based Fiber terminates, it gives control back to the top-most Fiber, not the Fiber which transfered to it. Is this intended? Here's a script to replicate the issue:

manager = Fiber.new do
  parent = Fiber.current
  worker = Fiber.new do
    puts "2. Begin Worker"
    parent.transfer
    puts "4. End Worker"
  end

  puts "1. Transfer 1"
  worker.transfer
  puts "3. Transfer 2"
  worker.transfer
  puts "5. Finished manager"
end

manager.transfer
puts "6. Finished script"

I expect the steps to print in order:

1. Transfer 1
2. Begin Worker
3. Transfer 2
4. End Worker
5. Finished manager
6. Finished script

But instead, 5. ... is skipped:

1. Transfer 1
2. Begin Worker
3. Transfer 2
4. End Worker
6. Finished script

I think that's because my worker fiber terminates and passes control back to the top-level Fiber.

Should it have passed control back to the manager? Or is there another way to make sure worker is terminated, and manager gets control?

Updated by rmosolgo (Robert Mosolgo) over 2 years ago Actions #1 [ruby-core:115880]

I see that Ruby 3.3 has Fiber#kill coming (https://github.com/ruby/ruby/pull/7823), which I think will solve my problem: I can manually .kill Fibers instead of running them until they return. But still, I'm interested to learn whether this is a bug or not!

Updated by ioquatix (Samuel Williams) over 2 years ago Actions #2 [ruby-core:115940]

My initial feeling is that this is not a bug.

Transferring to a fiber is a uni-directional operation and no information is expected to be kept about the caller. If that is what you desire, use resume.

There is nothing that prevents you from using resume and then transfer.

If you do desire to transfer back to a specific fiber, you can code to that, e.g.

manager = Fiber.new do
  parent = Fiber.current
  worker = Fiber.new do
    puts "2. Begin Worker"
    parent.transfer
    puts "4. End Worker"
  ensure
    # Exit:
    parent.transfer
  end

  puts "1. Transfer 1"
  worker.transfer
  puts "3. Transfer 2"
  worker.transfer
  puts "5. Finished manager"
end

manager.transfer
puts "6. Finished script"

I don't know if there is a better semantic worth trying to tease out of this, but the semantics of transfer are fairly straight forward and I'm not sure we should change it as the chance of breaking something is probably fairly high.

Updated by rmosolgo (Robert Mosolgo) over 2 years ago Actions #3 [ruby-core:115943]

Thanks for taking a look! I was going from the example in the docs (https://docs.ruby-lang.org/en/master/Fiber.html#method-i-transfer), where the return value of .transfer is used by the caller.

And .transfer does return to the caller, as long as the caller is the main Fiber, for example, this prints out a sequence of messages:

parent = Fiber.current
worker = Fiber.new do |initial_arg|
  puts initial_arg # => 2. Begin Worker
  puts parent.transfer("3. Resume Parent") # => 4. Resume Worker
  "5. Exit Worker"
end

puts "1. Start"
puts worker.transfer("2. Begin Worker")
# => "3. Resume Parent"
puts worker.transfer("4. Resume Worker")
# => "5. Exit Worker"
puts "6. Exit Script"

But it behaves differently when inside a surrounding, non-main Fiber:

manager = Fiber.new do |manager_init|
  puts manager_init # => 1. Start
  parent = Fiber.current
  worker = Fiber.new do |initial_arg|
    puts initial_arg # => 2. Begin Worker
    puts parent.transfer("3. Resume Parent")
    "5. Exit Worker"
  end

  puts worker.transfer("2. Begin Worker")
  # => "3. Resume Parent"
  puts worker.transfer("4. Resume Worker")
  # => "5. Exit Worker"

  "6. Exit Script" # This isn't printed
end

puts manager.transfer("1. Start")

# 1. Start
# 2. Begin Worker
# 3. Resume Parent
# 4. Resume Worker
# 5. Exit Worker
# 
# ... (6 doesn't print here)

Maybe the first example is a special case, since it's the main Fiber. I'd be happy to add a comment about that to the Fiber docs if that'd be helpful!

Updated by ioquatix (Samuel Williams) over 2 years ago Actions #4 [ruby-core:115944]

In think clarifying the documentation is a good idea.

And .transfer does return to the caller, as long as the caller is the main Fiber, for example, this prints out a sequence of messages:

I think that's just intuitively correct based on the semantics.

Actually, one could argue that a fiber that was transferred to, that exits, without an explicit transfer, should be considered exiting the thread completely and in your case terminating the program. In this case, Ruby is being a bit generous with "transferring back to the main fiber" IMHO. Transfer should be seen as a one way operation.

Updated by mame (Yusuke Endoh) 5 days ago Actions #6 [ruby-core:125902]

@ioquatix (Samuel Williams) I don't think the PR solves this issue. In the following example, worker 1 is abandoned in a suspended state:

manager = Fiber.new do
  parent = Fiber.current
  worker2 = nil
  worker1 = Fiber.new do
    puts "worker 1: start"
    worker2.transfer
    puts "worker 1: end"
  end
  worker2 = Fiber.new do
    puts "worker 2: start"
    parent.transfer
    puts "worker 2: end"
  end
  puts "manager: start"
  worker1.transfer # worker1 -> worker2 -> parent
  puts "manager: middle"
  worker2.transfer # worker2 -> (exit) -> parent
  puts "manager: end"
  # Now manager's prev is worker2, but it is already over, so it returns to the root fiber.
  # So "worker 1: end" is not reached.
end
puts "script: start"
manager.transfer
puts "script: end"

My understanding is that returning to the root fiber when a fiber started by transfer reaches its end is by design. This spec is already confusing, and I'm afraid this PR makes it even more confusing.

(IMO, the thread should be terminated when a transferred fiber reaches its end.)

As you said, I think the solution here is: "If that is what you desire, use resume."

Updated by ioquatix (Samuel Williams) 5 days ago · Edited Actions #7 [ruby-core:125905]

I think the crux of the issue here is that there is no way to do a final transfer (or raise that transfers) that goes back to a specific fiber, but it probably should be possible.

fiber = Fiber.new do
  # Do stuff
  
  # Final transfer is back to scheduler:
  scheduler.transfer
end

# from scheduler
fiber.transfer
fiber.alive? # false

Basically, in order to exit a fiber (for it to become dead) you have to rely on it exiting, but you don't have control over where it goes. I'm still not sure what the right solution here is but the above PR was just an experiment.

Resume/Yield may be a solution in some cases but there are some cases where I'm not sure it's possible.

Updated by ioquatix (Samuel Williams) 5 days ago · Edited Actions #8 [ruby-core:125906]

Actually, this behaviour isn't as bad as I thought it was:

f1 = Fiber.new do
  f2 = Fiber.new do
    puts "f2"
  end.transfer
  puts "f1"
end.resume 

f2
f1

I thought f2 would transfer back out to the main fiber, but it goes back to the nested resumed fiber.

In the case of a scheduler, the main point would be knowing if transfer will come back to this fiber or not, e.g. Fiber.current.resumed? may be a helpful interface.

Updated by ioquatix (Samuel Williams) 5 days ago · Edited Actions #9 [ruby-core:125907]

Okay, I spoke too soon, I think this clearly demonstrates the confusion:

def event_loop
  loop_fiber = Fiber.new do
    task = Fiber.new do
      puts "task"
    end
    # scheduler executes the task
    task.transfer
    puts "loop: after task"
  end
  loop_fiber.resume
  puts "outer: after loop"
end

puts "Works:"
event_loop

puts "Doesn't work:"
Fiber.new do
  event_loop
end.transfer

So even if you use resume, because of the outer transfer, it has confusing behaviour.

One solution is to always transfer back out:

def event_loop
  loop_fiber = Fiber.new do
    task = Fiber.new do
      puts "task"
      # Final transfer:
      loop_fiber.transfer
    end
    task.transfer
    puts "loop: after task #{task.alive?}"
  end
  loop_fiber.resume
  puts "outer: after loop"
end

But if you do this, the fiber is still alive? which is problematic.

There are two alternative options, IMHO:

  1. Fiber#exit(*arguments) (or Fiber#transfer!(*arguments) - same as Fiber#transfer but the current fiber is marked as dead.
  2. Fiber.exit(to = nil) - behaves the same as if exiting from the end of the Fiber's block, but transfers to the given fiber if specified (or the default if not). Possible implementation: https://github.com/ruby/ruby/pull/17619

Example possible usage:

def event_loop
  loop_fiber = Fiber.new do
    task = Fiber.new do
      puts "task"
      # Option 1:
      loop_fiber.exit
      # Option 2:
      Fiber.exit(loop_fiber)
    end
    task.transfer
    puts "loop: after task #{task.alive?}"
  end
  loop_fiber.resume
  puts "outer: after loop"
end

It's possible that option 2 reads slightly better.

Actions

Also available in: PDF Atom