-
Notifications
You must be signed in to change notification settings - Fork 254
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
Non-blocking reads with configurable timeouts #167
base: master
Are you sure you want to change the base?
Conversation
Build failing due to #166, but is green otherwise, which tells me we probably need better tests for these code paths. |
I like the idea of non-blocking I/O, and I agree this PR is a good small step for us to move in that direction. I'm less interested in exposing public API like How does this affect other rubies and platforms? Officially, I think we only have the bandwidth to maintain MRI rubies and Unix-like systems because that's what we're using, but where reasonable, I'd like to keep it easy for other contributors. I think we should keep exploring this route, but if it breaks jruby or windows, maybe we can wrap this functionality into a module. |
@mtodd still looking to get this in? |
Rewire
IO#read
calls originating from theNet::BER::BERParser
module to use non-blocking reads, depending onIO.select
to block until the socket is readable or times out, raising a timeout error (instead of blocking forever; though that is still the default).The internal and external APIs are likely to change (feedback welcome). Would welcome general thoughts or concerns for these kinds of changes. From the recent research I've been doing, this seems to be the most reliable, forward-compatible way to implement non-blocking reads (using
IO.select
, as opposed tosetsockopt
withSO_RCVTIMEO
).Pairing this with similar changes for
write
would be the logical next step, assuming this makes sense. It's worth considering making non-blocking/blocking calls configurable; this might be necessary since it's not always needed and is likely to behave in unexpected ways, so we'd not want that to be a surprise for anybody, or worse: have no recourse if it breaks their integration surprisingly.I'm still thinking through what the end-result this would afford us, especially in terms of how the
Net::LDAP
API can change (search_async
, for instance). This PR is pretty superficial (no major changes), but is a good step towards introducing more asynchronous, non-blocking API designs.Thoughts?
see #164
cc @jch @schaary