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
Make it possible to set SO_REUSEPORT to the server's socket #505
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm hesitant to accept this as is right now. I wasn't aware of this option, and it'll need to be researched closer so that the PR is reviewed consciously. But I'm leaving a few notes for the future reference.
One thought is whether we really need this to be toggleable. SO_REUSEADDR
cannot be switched on or off, for example. Why would it be any different for SO_REUSEPORT
which is essentially a feature of the same category?
Are there any cases when it'd be undesired to have SO_REUSEADDR
on by default?
Some googling suggests that it may be a good idea to have both of these options set on the socket simultaneously.
Extra reading for maintainers:
- https://www.unixguide.net/network/socketfaq/4.11.shtml
- https://lwn.net/Articles/542629/
- https://stackoverflow.com/a/14388707/595220
Extra questions:
- How does this affect the ephemeral sockets?
- How does this impact the abstract sockets?
In my opinion here is the key logical difference between
Turning it on by default can lead to a situation when two independent processes (e.g two different cherrypy/cheroot servers) accidentally open connections to the same port and address, set I will add better handling of some special cases:
|
c203f77
to
b05d84e
Compare
Sorry for the long delay. I updated my PR and resolved previously discussed issues. Also, here are some examples of port reuse implementations in Python web servers: Alternatively, if this feature is too expensive to maintain, I can send PR making |
3831a91
to
d8e4e03
Compare
This commit introduces `reuse_port` option for load-balancing purpose.
9de0ad0
to
6f2f80a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me. It seems to follow similar conventions of other servers and unblocks an important use case.
I find it a little awkward that REUSE_ADDR
is now used in two different locations (under re-use port for Windows and by default for non-ephemeral addresses on other platforms).
- Windows can use
SO_REUSEADDR
instead ofSO_REUSEPORT
;
I'm not sure I understand this motivation. Can you provide a reference for why SO_REUSEADDR
is appropriate on Windows when the user is requesting SO_REUSEPORT
?
187a9a5
to
fbd6912
Compare
Historically SO_REUSEADDR in Windows works as a combination of SO_REUSEADDR and SO_REUSEPORT. It's the reason this flag is skipped for Windows in IS_EPHEMERAL_PORT section (unintended port sharing could lead to the long debugging). At the same time Windows has a flag SO_EXCLUSIVEADDRUSE which apparently is "anti-SO_REUSEPORT". That special case can probably be handled in the IS_EPHEMERAL_PORT section, but that would spread reuse port logic even more: if not IS_EPHEMERAL_PORT:
"""Enable SO_REUSEADDR for the current socket.
Skip for Windows (has different semantics)
or ephemeral ports (can steal ports from others).
Refs:
* https://msdn.microsoft.com/en-us/library/ms740621(v=vs.85).aspx
* https://github.com/cherrypy/cheroot/issues/114
* https://gavv.github.io/blog/ephemeral-port-reuse/
"""
sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
if IS_WINDOWS and not reuse_port and hasattr(socket, "SO_EXCLUSIVEADDRUSE"):
sock.setsockopt(socket.SOL_SOCKET, socket.SO_EXCLUSIVEADDRUSE, 1) |
I'm not fixing that. |
β What kind of change does this PR introduce?
π What is the related issue number (starting with
#
)Resolves #504
β What is the current behavior? (You can also link to an open issue here)
Cheroot cannot be used in infrastructure where bind address for it allocated and kept by some different process.
main.py:
server.py:
β What is the new behavior (if this is a feature change)?
Cheroot can optionally re-use port:
main.py:
server.py:
π Contribution checklist:
(If you're a first-timer, check out
this guide on making great pull requests)
the changes have been approved
and description in grammatically correct, complete sentences
This change isβ