Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question about io_read #1

Open
singpolyma opened this issue Mar 1, 2022 · 12 comments
Open

Question about io_read #1

singpolyma opened this issue Mar 1, 2022 · 12 comments

Comments

@singpolyma
Copy link

I have a question about the logic in io_read at https://github.com/bruno-/fiber_scheduler/blob/main/lib/fiber_scheduler/selector.rb#L98

It seems that it considers the resource unavailable (-EAGAIN) if length == 0 and the read failed. This makes sense because if I try to read nothing and it fails, then it must not be possible to read. However the read does not use length at all! It uses maximum_size which looks likely to be related to length, but is not the same value. Should this perhaps instead says if maximum_size > 0 ?

@bruno-
Copy link
Owner

bruno- commented Mar 3, 2022

Hey,

the #io_read method does not have a conventional read-like behavior.
Check the docs for io_read, I think they will make everything clear.

https://docs.ruby-lang.org/en/3.1/Fiber/SchedulerInterface.html#method-i-io_read

@singpolyma
Copy link
Author

So, reading over that again, the issue I see is that if one calls IO#read when the fd has nothing ready at all, then io.read_nonblock will return :wait_readable and if length was zero then the code will return a hard failure -EAGAIN instead of waiting for something to become available.

In practise, copying this code into my fiber scheduler I see the above happen when running the specs, which causes them to fail.

@bruno-
Copy link
Owner

bruno- commented Mar 8, 2022

Yea, this is interesting.

So, the selector from this gem was taken from Samuel's io-event gem and slightly changed. I still put his copyright at the top of the file, because it's mostly Samuel's work.

The length > 0 branches were explicitly added to io-event just a few days before ruby 3.1 was released, see commit:

socketry/io-event@f173fad#diff-a12eb715e8d43c5484492317b228fc02363668ad6c4ac27e7a3e0f94cb26da18

@bruno-
Copy link
Owner

bruno- commented Mar 8, 2022

It's obviously a special case for something, I'm not sure what.

We should ask Samuel - can I ask him to add you to socketry slack?

@singpolyma
Copy link
Author

Sure, you can find my email for such invites, etc at https://singpolyma.net/

@taq
Copy link

taq commented Jan 6, 2023

Hey @bruno- !

Just checked that, with the basic example:

require "fiber_scheduler"
require "open-uri"

FiberScheduler do
  Fiber.schedule do
    URI.open("https://httpbin.org/delay/2")
  end

  Fiber.schedule do
    URI.open("https://httpbin.org/delay/2")
  end
end

Running with 3.1.0, it works ok, but with 3.2.0:

fiber_scheduler-0.13.0/lib/fiber_scheduler.rb:175:in `io_read': wrong number of arguments (given 4, expected 3) (ArgumentError)

Seems something changed with 3.2.0.

@aristotelesbr
Copy link

+1 @taq

Here it works if you add (:waiting) to the block. 🤷🏻‍♂️

require "fiber_scheduler"
require "open-uri"

FiberScheduler do
  Fiber.schedule(:waiting) do
    URI.open("https://httpbin.org/delay/2")
  end
end

@taq
Copy link

taq commented Jan 10, 2023

Nice @aristotelesbr !

Will try here and take a look why on 3.2 it is needed.

@kingdonb
Copy link

kingdonb commented Feb 26, 2023

Can someone please elaborate on what Fiber.schedule(:waiting) is going to change, I blindly applied this fix to my fibers app to try to get it to run on 3.2.1

And while it does run with this change, it does not "run"

The fibers are not firing anymore. Is there a corollary command to run later, which should start the fibers that are waiting?

(I'm rolling back to Ruby 3.1.3 for now, to get the fibers running again)

I'm sure I can find more information about this myself searching, but it's the third time I've had to come back to this thread so I thought it might actually be helpful to ask for clarification at this point!

edit: Code
kingdonb/simplest-commitbee#25

@taq
Copy link

taq commented Feb 27, 2023

Hey @kingdonb

I still didn't have time to take a look at this, but seems some method calls changed on 3.2. I noticed it prior to give a Ruby online class where I showed Fibers.

@taq
Copy link

taq commented Feb 28, 2023

Hey @bruno-

I checked where the error is, and it's here on rvm/gems/ruby-3.2.0/gems/fiber_scheduler-0.13.0/lib/fiber_scheduler.rb:175:

  def io_read(io, buffer, length)
    @selector.io_read(Fiber.current, io, buffer, length)
  end

but the current code here on Github on the same file is

def io_read(fiber, io, buffer, length, offset = 0)

which handle the offset as the last parameter, making it work with Ruby 3.2.0 as the commit here provides: 316d5aa

So, seems we just need a new gem release. :-)

@taq
Copy link

taq commented Mar 14, 2023

@bruno- if you need some help, just let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants