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
fantix added a commit that referenced this pull request Oct 14, 2024
Changes
=======

* Add cleanup_socket param on create_unix_server() (#623)
  (by @fantix in d6114d2)

Fixes
=====

* Use cythonized SO_REUSEPORT rather than the unwrapped native one. (#609)
  (by @ptribble in 4083a94 for #550)

* UDP errors should result in protocol.error_received (#601)
  (by @jensbjorgensen in 3c3bbef)

* Updates for Cython3 (#587)
  (by @alan-brooks in 3fba9fa for #587)

* Test with Python 3.13 (#610)
  (by @edgarrmondragon in fb5a139)
@fantix fantix mentioned this pull request Oct 14, 2024
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