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

SSL: supporting async connection establishment #20

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dorszman
Copy link

@dorszman dorszman commented Mar 9, 2018

SSL connection handshake is not async, as a result calling add() to initiate a new https request results in blocking till ssl connection is fully established. This may take a couple of hundreds of ms.
The proposed fix - make ssl connection fully async and non-blocking.

@anall
Copy link

anall commented Jun 3, 2019

I am in the process of taking over this project, and I apologize for how long this pull request has gone without response.

This pull request breaks for non-SSL requests:

Can't locate object method "connect_SSL" via package "Net::HTTP::NB" at lib/HTTP/Async.pm line 940.

Also on a server that never accepts on the incoming socket, this change removes the (timeout-long) delay on add, but also does not seem to respect the timeout on wait_for_next_response. The timeout is respected in wait_for_next_response on HTTP requests w/o your commit.

Given the delay in responding, are you still interested in working on this pull request and fixing these issues? If not, do you have any objection for me using this as a starting point?

@dorszman
Copy link
Author

dorszman commented Jun 4, 2019 via email

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

Successfully merging this pull request may close these issues.

2 participants