socketry / async-io

Concurrent wrappers for native Ruby IO & Sockets.
MIT License
209 stars 28 forks source link

async-io `Async::IO::Endpoint#connect` should work without Async reactor #56

Open mrhardikjoshi opened 1 year ago

mrhardikjoshi commented 1 year ago

Hello Everyone,

I am using echo/client.rb example without having Async wrapper call around it. I am aware some of this gem's feature like SharedEndpoint and Trap won't work without Async reactor, But I am focused on Endpoint connect/close, read/write and similar operations. As per my understanding those do not require to be run within Async reactor.

Following is how my changed example of echo/client.rb looks like:

require 'async'
require 'async/io/host_endpoint'
require 'async/io/stream'

endpoint = Async::IO::Endpoint.tcp('localhost', 4578)
# peer = endpoint.connect        # doesn't work
peer = Sync { endpoint.connect } # Works
stream = Async::IO::Stream.new(peer)

while true
  sleep 1
  stream.puts "Hello World!"
  puts stream.gets.inspect
end

This doesn't works when endpoint.connect is not wrapped in Async/Sync block. With following error

$ruby test.rb
.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/async-2.2.1/lib/async/task.rb:178:in `current': No async task available! (RuntimeError)
    from .rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/async-io-1.34.0/lib/async/io/host_endpoint.rb:55:in `connect'
    from test.rb:8:in `<main>'

Above exception is raised here while trying to get the current task task = Task.current. This current task is only used for annotation here and here. (I didn't use local address to bind so it won't need task to created another fiber => reference).

So my question: Is it correct to use Async::IO without Async reactor like above? and Should we change Async::IO::Endpoint#connect and Async::IO::Socket.connect methods to get the current task without raising exception and work outside of reactor too? If yes then I am happy to make a PR with these changes.

Thank you :)

webgago commented 1 year ago

Hey @mrhardikjoshi,

Pretty much everything related to async ecosystem should be run inside a reactor.

You can check how it works in falcon server, ex. https://github.com/socketry/falcon/blob/main/lib/falcon/command/supervisor.rb#L81

Async do
  endpoint.connect do |socket|
    stream = Async::IO::Stream.new(socket)
    # your code here
  end
end
ioquatix commented 1 year ago

Thanks for your question and thanks @webgago for your answer, which is mostly correct.

That being said, there is no reason why the endpoint abstraction needs to run in async, and I'm working on extracting it out of Async::IO in the future, but there are some missing nice to have features in core Ruby which make removing the Async dependency a little painful right now.

In the mean time, you CAN use Addrinfo or TCPServer if you prefer native interfaces. As long as you run it inside an Async block, it will be event driven on Ruby 3.1+.

Regarding whether these things should create an async block, it's a little bit complicated. Because connect and bind and other operations sometimes involve more than one socket... it is hard to know where the task boundary should exist. I think it's better if IO::Endpoint doesn't need to know about async task, and in the future I'd like it if it didn't know about async either and works on any fiber scheduler.