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

Use cythonized SO_REUSEPORT rather than the unwrapped native one. #609

Merged
merged 3 commits into from
Aug 16, 2024

Conversation

ptribble
Copy link
Contributor

The code already detects (via hasattr) whether SO_REUSEPORT is available, setting has_SO_REUSEPORT, and creates a wrapped SO_REUSEPORT (via getattr). This change ensures it's that wrapped SO_REUSEPORT that's used, rather than the native C version (which may not exist).

Fixes #550

@@ -1775,7 +1775,7 @@ cdef class Loop:
if reuse_address:
sock.setsockopt(uv.SOL_SOCKET, uv.SO_REUSEADDR, 1)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't the same problem here with SO_REUSEADDR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No; the place to look is uvloop/includes/stdlib.pxi

cdef int has_IPV6_V6ONLY = hasattr(socket, 'IPV6_V6ONLY')
cdef int IPV6_V6ONLY = getattr(socket, 'IPV6_V6ONLY', -1)
cdef int has_SO_REUSEPORT = hasattr(socket, 'SO_REUSEPORT')
cdef int SO_REUSEPORT = getattr(socket, 'SO_REUSEPORT', 0)

Note that there's a check to see if the variable exists, and then a python definition of the variable with a default, guaranteeing it's defined. This is how the code handles platforms that don't have the constants defined. For other constants (such as SO_REUSEADDR) there's neither a check nor a python definition of the constant.

Copy link
Member

@fantix fantix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's also drop the definition of uv.SO_REUSEPORT to prevent future usage.

@fantix fantix merged commit 4083a94 into MagicStack:master Aug 16, 2024
11 checks passed
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

Successfully merging this pull request may close these issues.

uvloop strictly needs SO_REUSEPORT
3 participants