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

Discussion: should trio.ssl be imported by default? #199

Closed
njsmith opened this issue Jun 11, 2017 · 4 comments
Closed

Discussion: should trio.ssl be imported by default? #199

njsmith opened this issue Jun 11, 2017 · 4 comments

Comments

@njsmith
Copy link
Member

njsmith commented Jun 11, 2017

Currently it is. Glyph says that's bad, because there are cases where you have lots of tiny processes stuffed onto a box and openssl is kinda memory hungry, so it's better not to load the ssl module until you have to.

I'm not super-opposed to making people import trio.ssl, but if we have a high-level trio.open_ssl_tcp_stream(...) function then it may take some awkward work to avoid importing it early. And I'm not a big fan of the idea that calling that function would implicitly cause trio.ssl to materialize in the user-visible namespace.

@kyrias
Copy link
Contributor

kyrias commented Jun 11, 2017

Might make sense to either have the open function be part of the ssl module, alternatively have a function for "loading" the ssl module that's required to be run before you're allowed to run open_ssl_tcp_stream. Not sure how nice that would be in practice though.

@njsmith
Copy link
Member Author

njsmith commented Jun 11, 2017

Might make sense to either have the open function be part of the ssl module

That's a plausible option too, yeah, good point.

It looks like twisted is not terribly worried about this – if you use their endpoint module at all (which is the recommended way to do all networking) then they unconditionally load openssl.

@njsmith
Copy link
Member Author

njsmith commented Jun 11, 2017

It's also possible to make trio.ssl be auto-loaded the first time its referenced. It's not ideal, because of the added magic factor (people don't expect accessing an attribute to trigger disk access; any errors are delayed until a weird and unexpected place), but it wouldn't be the end of the world either.

Before doing anything drastic it's probably also worth checking how much memory openssl actually uses if you haven't called SSL_new. Merely loading the library shouldn't be a big deal, that's just a chunk of mmap'ed memory that (a) is shared between processes and (b) doesn't need to actually be loaded into RAM at all. But it's possible they allocate some memory at load time or something, I don't know.

@njsmith
Copy link
Member Author

njsmith commented Jan 25, 2019

trio.ssl is gone now (#852), so this is just about how to handle the top-level SSLStream, SSLListener, open_ssl_over_tcp_stream, open_ssl_over_tcp_listeners. And I guess there are two reasonable options:

  • Try to import them at startup, but if this produces an ImportError because the ssl module is missing, then shrug and move on (like twisted does)
  • Lazily import ssl the first time its needed

Neither approach is terribly scary. I guess we can close this until someone runs into the issue for real, and then we can ask them which approach they prefer.

@njsmith njsmith closed this as completed Jan 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants