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

Add #wait_readable and #wait_writable to IO::FileDescriptor and Socket #15377

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

ysbaddaden
Copy link
Contributor

@ysbaddaden ysbaddaden commented Jan 27, 2025

Follow up to #15376 that exposes the #wait_readable and #wait_writable methods to IO::FileDescriptor and Socket directly.

I'm not sure this is the actual way to go. This is mostly an example to get started discussing on how to go forward for a more public API for #15374 and the ideas from RFC #0007:

@ysbaddaden ysbaddaden force-pushed the feature/add-io-wait-ready-methods branch from de3efdd to 930d521 Compare January 27, 2025 14:01
@will
Copy link
Contributor

will commented Feb 11, 2025

I don't have an opinion on what the public API should be here, but just wanted to tag @stakach here since I know this is a problem with libssh2 spider-gazelle/ssh2.cr#22

@stakach
Copy link
Contributor

stakach commented Feb 11, 2025

wait_readable / wait_writable work for me
should be possible to implement on Windows too for the socket side of things, it's not possible with files on windows
https://gist.github.com/stakach/4d68e19e14feeaf251686ae6aaa54b77

Not that I use libssh2 on windows but it might be useful for someone and implementing the socket checks for windows would make it possible

@ysbaddaden
Copy link
Contributor Author

I fixed a couple details.

Still, I'm wondering if exposing these methods is a good idea. I can't shake the feeling that we could have a more interesting take. For example an IO::Select object that could wait on IO objects in general, or having some integration with the select keyword...

For now, I keep this PR as a draft. The methods are available on the event loop:

evloop = Crystal::EventLoop.current
evloop.wait_readable(socket || file_descriptor)
evloop.wait_writable(socket || file_descriptor)

@stakach
Copy link
Contributor

stakach commented Feb 20, 2025

Being able to wait on IO objects in general (like an array of sockets) would be ideal and integrating that with select even more so

Need to be able to handle adding or removing sockets from the array too, possibly a channel could be used in the select to break you out of it, allowing you to re-select, something like

@sockets : Array(IPSocket)
update_sockets_notify = Channel(Nil).new

# ...

loop do
  select
  when socket = IO::Select.wait_readable(@sockets)
    process_io(socket)
  when update_sockets_notify.receive
    # no-op, notifying of a change to the @sockets array
  end
end

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

Successfully merging this pull request may close these issues.

3 participants