Bug #20413: Enumerator can block fiber scheduler.

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

[ioquatix (Samuel Williams)]

Using Enumerator in the event loop can cause problems as the fiber created by rb_fiber_new is blocking by default. It should be non-blocking.

#!/usr/bin/env ruby

require 'async'

Async do
  Async do
    while true
      puts "Hello"
      sleep 1
    end
  end
  
  enumerator = Enumerator.new do |yielder|
    while true
      yielder << "World"
      sleep 1
    end
  end
  
  while true
    puts enumerator.next
  end
end

Before this PR, the output is:

> ./test.rb
Hello
World
World
World
World
World
World
...

After this PR, the output is:

> ./test.rb
Hello
World
Hello
World
Hello
World
Hello
World
Hello
World
...

The reason why this happens, is because the enumerator sleep never yields to the event loop.

See https://github.com/ruby/ruby/pull/10481 for the fix.

[ioquatix (Samuel Williams)]

• Description updated (diff)

[Eregon (Benoit Daloze)]

Changing rb_fiber_new sounds incompatible: https://github.com/ruby/ruby/pull/10481#issuecomment-2041432435
Changing Enumerator fibers would be safer, but probably still incompatible.

What is the use-case here, to use an Enumerator backed by a Fiber (so next/peek) in a Fiber scheduler?

[ioquatix (Samuel Williams)]

What is the use-case here, to use an Enumerator backed by a Fiber (so next/peek) in a Fiber scheduler?

Yes.

Changing Enumerator fibers would be safer, but probably still incompatible.

Can you show evidence where this breaks some existing test or program?

The fiber scheduler is transparent to user code, so there shouldn't be any incompatibility. Additionally, Fiber.new adopted this a while ago and there have been no issues. Fiber.new is arguably more widely used than rb_fiber_new.

[ioquatix (Samuel Williams)]

• Description updated (diff)

[ioquatix (Samuel Williams)]

After discussing it with Eregon on Slack, he withdrew his objection to this change.

In general, this change:

• Aligns Fiber.new with rb_fiber_new so that they both produce non-blocking fibers.
• Doesn't affect existing code, as there is no obvious usage of rb_fiber_new by GitHub code search (small/zero blast radius).
• Only impacts usage within the fiber scheduler, i.e. no effect outside of fiber scheduler beside the predicate value itself.
• Even within the fiber scheduler, it is transparent to user code.