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

Maybe we need to investigate non-blocking connect with timeout #409

Closed
ioquatix opened this issue Nov 16, 2020 · 2 comments
Closed

Maybe we need to investigate non-blocking connect with timeout #409

ioquatix opened this issue Nov 16, 2020 · 2 comments

Comments

@ioquatix
Copy link
Member

redis/redis-rb#960 (comment)

@rhenium
Copy link
Member

rhenium commented Feb 17, 2021

Sorry for the delayed response.

I also noticed this:

          begin
            # Initiate the socket connection in the background. If it doesn't fail
            # immediately it will raise an IO::WaitWritable (Errno::EINPROGRESS)
            # indicating the connection is in progress.
            # Unlike waiting for a tcp socket to connect, you can't time out ssl socket
            # connections during the connect phase properly, because IO.select only partially works.
            # Instead, you have to retry.
            ssl_sock.connect_nonblock
          rescue Errno::EAGAIN, Errno::EWOULDBLOCK, IO::WaitReadable
            if ssl_sock.wait_readable(timeout)
              retry
            else
              raise TimeoutError
            end
          rescue IO::WaitWritable
            if ssl_sock.wait_writable(timeout)
              retry
            else
              raise TimeoutError
            end
          end

I don't have time to investigate WHY this was a problem previously, but I don't know if the retry is needed or not.

I may be missing the point, but it's usually necessary to call #connect_nonblock more than twice since a TLS handshake typically takes 2 RTT. The timeout value should be handled better, though, similarly to how net/* stdlib does.

https://github.com/ruby/net-protocol/blob/8ecb835677e79011bbf470de20fdb9f4b2d6f833/lib/net/protocol.rb#L40-L56

@rhenium
Copy link
Member

rhenium commented Dec 20, 2021

I don't get what exactly was the question, but ruby-openssl should not block at all if the application uses *_nonblock variant all the time.

I'm closing this issue.

@rhenium rhenium closed this as completed Dec 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants