Check to see if SO_REUSEPORT is usable and not just defined.

[sethmlarson]

Some platforms define SO_REUSEPORT but do not implement it, resulting in cryptic OSErrors when trying to use socket.SO_REUSEPORT. This PR would solve that issue for all platforms that implement but don't define a usable SO_REUSEPORT. More info in Python issue 26858. See issue #352 .

Edit: I have signed the PSF Contributor Agreement

[gvanrossum]

Why can't you use just if not _HAS_USABLE_SO_REUSEPORT:?

(Also, please don't use \ for continuations, use an extra pair of (). PEP 8 please!

[sethmlarson]

Because of all the mock patching in the test cases. Patching the socket module in the *_test_nosoreuseport prevents that atm. And I will change the bracing.

[sethmlarson]

Switched the order of the check, should still pass tests but also will not have to check hasattr() each call in real use.

[gvanrossum]

I think those tests need to be fixed then. There should be only one way to test for SO_REUSEPORT.

[sethmlarson]

@gvanrossum Alternative is to do the check inline when the reuse_port option is specified? Wouldn't be a performance hit as create_server and create_unix_server probably get used once and forgotten.

[gvanrossum]

Hm, maybe just catch the exception and translate it into the same ValueError (actually a slightly similar message so we can tell the difference).

[sethmlarson]

@gvanrossum I switched to catching the exception inline, I also added a test to test an ill-defined SO_REUSEPORT as with the issue this PR fixes.

[gvanrossum]

LGTM now. @1st1 what do you think?

[1st1]

LGTM. Can we merge this after b1?

[sethmlarson]

This also closes issue #352 as mentioned above. Can be closed if accepted but awaiting merge?

[gvanrossum]

Will merge (here and in CPython) after 3.6b1.

[1st1]

Pushed in 610c03f. Thanks!

[1st1]

See also https://hg.python.org/cpython/rev/c1c247cf3488/. Looks like this is your first commit to CPython, congrats if so!

BTW, did you sign Python Contributor Agreement?

[sethmlarson]

@1st1 Thank you! This is my first commit and I signed it the day I submitted this PR. See my comment above in the PR.

[Dakkaron]

I am sorry, but to me this fix actually causes more problems than it fixes.

Yes, the old error message is rather cryptic and the new exception message is way better, but changing the exception type seems wrong to me. Yes, ValueError might fit better, but changing the type will cause a lot of problems.

• Since an exception is raised as before, code that does not catch anything here will still fail.
• Old code that caught the old OSError is now broken, since it does not catch ValueError.
• New code that catches only ValueError will fail on older versions of Python, since it does not catch OSError
• From what I can see, the documentation mentions neither OSError nor ValueError, so if a programmer wants to make sure they have proper exception handling for that function, they must check the source, which, depending on their Python version, will say either OSError or ValueError, which will break if used on a different version of Python.

My recommendation would be to catch the old OSError and then raise a new OSError with the improved error message.

[sethmlarson]

@Dakkaron I made this change for consistency reasons so that trying to use reuse_port on a unsupporting platform and a platform that advertises support but is actually horribly broken raise the same error type. I didn't see the value in having to catch two different error types for a very similar situation (ie you can't use SO_REUSEPORT).

If you think this should be changed you can raise an issue on the Python tracker.

[Dakkaron]

Sorry, my bad, I didn't notice that the old version already raised a ValueError, so your version just removes one exception type. So your change does actually help.

[sethmlarson]

Glad I could clarify. :)