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

Add option to create reusable sockets #3

Closed
bablokb opened this issue Mar 15, 2024 · 7 comments
Closed

Add option to create reusable sockets #3

bablokb opened this issue Mar 15, 2024 · 7 comments

Comments

@bablokb
Copy link

bablokb commented Mar 15, 2024

According to the release-notes for CP 9.0.0:

  • Incompatible change: Require explicit socket port reuse. Use socket.setsockopt(pool.SOL_SOCKET, pool.SO_REUSEADDR, 1), as in CPython.

you have to explicitly mark sockets for reuse. Currently, user code has to create a session, call get and then manipulate the socket from the Response-object.

I think it would be much simpler to have this rather low-level manipulation of sockets as part of ConnectionManager. Or even better, as part of socketpool as the factory of sockets. But the latter would need changes to CP itself.

@justmobilize
Copy link
Collaborator

@tannewt do you think this would be a good idea? Where we set the timeout, we could also check the CP version and if it's greater than 9, set this option

@anecdata
Copy link
Member

anecdata commented Mar 15, 2024

I've been wrestling with trying to understand the socket reuse issue, there's a lot of history in the core issues and PRs, and a lot of unclear info out on the ethers.

My understanding is that SO_REUSEADDR used to be always set for listen-ing sockets implicitly, but now it needs to be done explicitly (better Python compatibility). But is setting SO_REUSEADDR something that only applies to servers / listen, or would there be benefit (or at least no side effects) of setting it for all sockets?

@dhalbert
Copy link
Contributor

Yes, SO_REUSEADDR would be set for server-side sockets. You'd set it before bind(), which is before listen(). I don't know of a purpose for using it on the client set, in adafruit_requests.

Server examples that set SO_REUSEADDR:
https://www.codementor.io/@joaojonesventura/building-a-basic-http-server-from-scratch-in-python-1cedkg0842#sending-http-responses-using-sockets
https://medium.com/@stephen.biston/write-an-http-server-from-the-ground-up-in-9-minutes-with-python-1fdb9800a26a
etc.

@justmobilize
Copy link
Collaborator

Ahh, so we wouldn't add that here, unless we added support for getting server-side sockets in connection manager

@anecdata
Copy link
Member

I was actually wondering about that this morning... was going to see if CM meshed with adafruit_http_server.

@dhalbert
Copy link
Contributor

Ahh, so we wouldn't add that here, unless we added support for getting server-side sockets in connection manager

That does not seem necessary to me. adafruit_httpserver takes a socket pool as an argument and takes care of getting a socket and setting it for reuse: https://github.com/adafruit/Adafruit_CircuitPython_HTTPServer/blob/dc9f83cd1a212520f4728dc3025dc881ffad3035/adafruit_httpserver/server.py#L207. That is typically the server's job.

I would suggest closing this issue unless, @bablokb, you have a particular use case in mind. The point of connection manager to me is to wrap and hide the multiple sources of socket pools and ssl contexts.

@bablokb
Copy link
Author

bablokb commented Mar 15, 2024

Thanks for the clarification, it was not clear for me that this is not an issue for the client side.

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