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

[WIP] Convert async I/O backend to Socketry #377

Closed
wants to merge 1 commit into from
Closed

Conversation

tarcieri
Copy link
Member

@tarcieri tarcieri commented Sep 24, 2016

Socketry uses async I/O to implement thread-safe timeouts:

https://github.com/socketry/socketry

cc @ixti @zanker @TiagoCardoso1983

reset_timer
rescue IOError, SocketError, SystemCallError => ex
rescue Socketry::Error => ex
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are probably more places this sort of change needs to be made

# @param (see #initialize)
# @return [void]
def start_tls(req, options)
return unless req.uri.https? && !failed_proxy_connect?
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why the failed_proxy_connect? call is needed here, but I'm guessing it's related to some of the SSL + proxy test failures.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect it is, I didn't add that at least.

@tarcieri
Copy link
Member Author

tarcieri commented Sep 24, 2016

Note: Socketry's timeouts can subsume HTTP::Timeout::Null and HTTP::Timeout::PerOperation, but HTTP::Timeout::Global will need to be (re)implemented.

Some changes to the timeout API will probably be necessary.

@tarcieri
Copy link
Member Author

Note per cb420da this is a breaking change which will require a new major release (3.x) as among other things it will require Ruby 2.2+ due to the Socketry dependency.

Since Rails 5 is also 2.2+, hopefully this is ok.

@tarcieri tarcieri force-pushed the socketry branch 5 times, most recently from 47425bd to 39473ae Compare November 26, 2016 03:02
@tarcieri
Copy link
Member Author

Well, still a WIP (especially timeout support) but it's passing consistently on 2.3 and JRuby 9.1.6.0 (and sporadically on 2.2)

@tarcieri tarcieri force-pushed the socketry branch 2 times, most recently from 75456c5 to abcccba Compare November 26, 2016 08:54
Socketry is a high-level socket library which uses async I/O to implement
thread-safe timeouts:

https://github.com/socketry/socketry

Socketry requires Ruby 2.2.6+

JRuby version is bumped to 9.1.6.0 as JRuby has various async I/O bugs on
earlier versions, and this is the first release that seems solid async I/O-wise
@tarcieri
Copy link
Member Author

If anyone wants to resume work on this, let me know.

@tarcieri tarcieri closed this Mar 11, 2019
@tarcieri tarcieri deleted the socketry branch March 11, 2019 19:34
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