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

Send Trait for Connection #32

Closed
sgessa opened this issue Apr 8, 2019 · 3 comments
Closed

Send Trait for Connection #32

sgessa opened this issue Apr 8, 2019 · 3 comments

Comments

@sgessa
Copy link

sgessa commented Apr 8, 2019

I'm trying to use this library from a multi-threaded context.
Is there any particular reason the Connection struct can't implement Send?

@ghedo
Copy link
Member

ghedo commented Apr 8, 2019

One problem is that the TLS handshake state (which is a raw pointer to a BoringSSL SSL object) is not thread-safe, which makes the whole Connection not thread-safe as well. It might be possible to make it thread-safe by wrapping the TLS state in Arc<Mutex<...> but that would need to be tested to make sure it doesn't break any assumptions made by BoringSSL.

@puckipedia
Copy link

I believe impling Send would be safe on both Context and Handshake, as that just means allowing the respective structs to move between threads, which is okayed by the ssl.h comments:

// An |SSL| object represents a single TLS or DTLS connection. Although the
// shared |SSL_CTX| is thread-safe, an |SSL| is not thread-safe and may only be
// used on one thread at a time.

@ctiller
Copy link
Contributor

ctiller commented Mar 30, 2020

One would also need to deal with application data on streams - currently this is a Box<dyn std::any::Any> and it would need to be Box<dyn Send + std::any::Any>, which would break the current ffi interface, since it's taking a *mut c_void for application data, which is not Send.

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