ruby-net-ldap icon indicating copy to clipboard operation
ruby-net-ldap copied to clipboard

Non-blocking reads with configurable timeouts

Open mtodd opened this issue 9 years ago • 3 comments

Rewire IO#read calls originating from the Net::BER::BERParser module to use non-blocking reads, depending on IO.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).

  • [x] non-blocking reads
  • [x] configurable timeout
  • [ ] extensive unit & integration tests

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 to setsockopt with SO_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 https://github.com/ruby-ldap/ruby-net-ldap/pull/164 cc @jch @schaary

mtodd avatar Dec 01 '14 07:12 mtodd

Build failing due to #166, but is green otherwise, which tells me we probably need better tests for these code paths.

mtodd avatar Dec 01 '14 07:12 mtodd

Reading material (some modern, recent, relevant, and others not quite so fresh):

  • http://spin.atomicobject.com/2013/09/30/socket-connection-timeout-ruby/
  • http://mikeperham.com/2009/03/15/socket-timeouts-in-ruby/
  • http://stackoverflow.com/questions/9853516/set-socket-timeout-in-ruby-via-so-rcvtimeo-socket-option/12111120#12111120
  • http://stackoverflow.com/questions/9853516/set-socket-timeout-in-ruby-via-so-rcvtimeo-socket-option

mtodd avatar Dec 01 '14 07:12 mtodd

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 search_async, instead preferring non-blocking I/O concerns to be internal and private. I don't think net-ldap users should know how I/O is implemented.

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.

jch avatar Dec 15 '14 18:12 jch