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

Support rust-openssl 0.9 #36

Open
pimeys opened this issue Feb 16, 2017 · 5 comments
Open

Support rust-openssl 0.9 #36

pimeys opened this issue Feb 16, 2017 · 5 comments

Comments

@pimeys
Copy link

pimeys commented Feb 16, 2017

Hi,

I've been trying to update solicit to support rust-openssl 0.9, and I'm almost there. The last issue is to implement try_split for SslStream<TcpStream>, but the underlying method try_clone for SslStream was deprecated in rust-openssl 0.8.

Has anybody else tried to update this? One option is to go with Arc<Mutex<_>>, but I'd like to know is there any simpler solution here...

Ping @jwilm et.al.

@jwilm
Copy link
Contributor

jwilm commented Feb 16, 2017

Arc<Mutex<_>> would solve the API issue certainly, but the current async client architecture assumes both the reader and writer threads can block on read/write in parallel. That's not going to be possible with a mutex.

Updating the async client turns out to be complex as a result. The right solution is to use mio or tokio to multiplex I/O. Both @mlalic and I have made an attempt at building such a client (based on mio), but in both cases we ran out of time and/or motivation.

@pimeys
Copy link
Author

pimeys commented Feb 16, 2017

Solicit is kind of blocking the release of our apns2[0] library, which would be a total PITA to include in any project (we needed to fork solicit to support at least rust-openssl 0.7).

I found there's some effort to do an async http2 client with tokio[1], but no updates for the past two months. Would be very useful with apns, but also having some lack of time/motivation...

[0] https://github.com/polyitan/apns2
[1] https://github.com/lambdastackio/tokio-http2

@mlalic
Copy link
Owner

mlalic commented Feb 16, 2017

On the topic of an async client, I have a new crate based on Tokio rather than raw mio: tokio-solicit. This actually didn't require too much work at all to get going. The only thing missing for feature parity [0] with the client in this crate is ALPN negotiation, which requires some work on tokio-tls dependencies (namely, rust-native-tls, of which my fork is here [1]).

As for the question at hand... I don't think that even the forced synchronization that you're suggesting would actually work. Sure, it would compile, but making a blocking read while holding the mutex? It would mean you couldn't start new writes in parallel, so the whole point of "splitting" the socket in two halves would be lost. In all honesty, the client is fairy inefficient (uses way too many threads) and wasn't really meant to be the main thing that this crate exports, though it seems that's how it worked out. :) [2]

My philosophy here was that HttpConnection (and the ClientConnection/ServerConnection helpers) should not depend on any concrete IO implementation -- and they don't. However, the clients, of course, do; they were simply provided for use by folks who really don't need to do anything special at all...

So, you could go the route of implementing ReceiveFrame and SendFrame yourself on top of the IO primitive that you do have and simply providing these instances to the connection. However, this doesn't absolve you of thinking about synchronization. And the way I see it, if the socket is blocking and if you can't independently wait on both read and write completion on it, the only solution is evented/async IO, as HTTP/2 really needs to react to completely async events happening on the stream (i.e. not only reading from the socket after sending a request and going to "sleep" after the response is read).

[0] Or even surpassing it, as it already supports streaming responses

[1] This could be upstreamed if the maintainers are open to (at least temporarily) not having each platform be "equal", i.e. only openssl would have ALPN to start with. I could personally get it done for Windows SChannel-based TLS as well, but I'm not familiar with the OSX APIs at all.

[2] I'm hoping the async client can completely supersede all of these, so that I can even remove them from this crate altogether, but that's a whole different thing...

@sfackler
Copy link

[1] This could be upstreamed if the maintainers are open to (at least temporarily) not having each platform be "equal", i.e. only openssl would have ALPN to start with. I could personally get it done for Windows SChannel-based TLS as well, but I'm not familiar with the OSX APIs at all.

The entire point of native-tls is uniform support across platforms, and ALPN is not part of Secure Transport's public API at the moment. You'll probably be better off hitting tokio-openssl directly.

@mlalic
Copy link
Owner

mlalic commented Feb 17, 2017

@sfackler Well, that's a real bummer. I do get that the idea is for all platforms to be equal, but I thought there wouldn't really be any harm in making it work for OpenSSL and Windows behind a feature, until Secure Transport catches up. I guess at that point, it's almost the same making a different wrapper crate anyway, though.

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

No branches or pull requests

4 participants