bpo-33408: Enable AF_UNIX support in Windows #14823
Conversation
Lib/socket.py
Outdated
| @@ -494,11 +494,14 @@ def socketpair(family=None, type=SOCK_STREAM, proto=0): | |||
| else: | |||
|
|
|||
| # Origin: https://gist.github.com/4325783, by Geert Jansen. Public domain. | |||
| def socketpair(family=AF_INET, type=SOCK_STREAM, proto=0): | |||
| def socketpair(family=AF_UNIX, type=SOCK_STREAM, proto=0): | |||
There was a problem hiding this comment.
This causes test failures in appveyor and Azure Pipelines. Must be refactored
| if family == AF_INET: | ||
| host = _LOCALHOST | ||
| elif family == AF_INET6: | ||
| host = _LOCALHOST_V6 | ||
| elif family == AF_UNIX: | ||
| from uuid import uuid4 | ||
| host = os.path.join(os.environ['TEMP'], str(uuid4())) | ||
| else: | ||
| raise ValueError("Only AF_INET and AF_INET6 socket address families " |
| if family == AF_INET: | ||
| host = _LOCALHOST | ||
| elif family == AF_INET6: | ||
| host = _LOCALHOST_V6 | ||
| elif family == AF_UNIX: | ||
| from uuid import uuid4 | ||
| host = os.path.join(os.environ['TEMP'], str(uuid4())) |
There was a problem hiding this comment.
I'm really not a fan of this - can we use unnamed sockets instead?
There was a problem hiding this comment.
Abstract sockets (starting from NULL in the name) is another options.
Web says that Windows supports it.
| @@ -529,6 +543,8 @@ def socketpair(family=AF_INET, type=SOCK_STREAM, proto=0): | |||
| raise | |||
| finally: | |||
| lsock.close() | |||
| if family == AF_UNIX: | |||
| os.unlink(host) | |||
There was a problem hiding this comment.
Yeah, I don't like this at all :)
| self.set_reuse_addr() | ||
| if sys.platform == 'win32' and family == socket.AF_UNIX: | ||
| # calling set_reuse_addr() on Windows with family AF_UNIX results in: | ||
| # OSError: [WinError 10045] The attempted operation is not supported for the type of object referenced |
There was a problem hiding this comment.
set_reuse_addr seems to handle all OSError silently - why do we need to skip it here?
| self.assertIn(self.sock.getsockname(), ('', None)) | ||
| if sys.platform == 'win32': | ||
| # Getting the name of unbound socket on Windows | ||
| # raises an exception |
There was a problem hiding this comment.
Does this make more sense than returning None? Which is apparently a valid return value
| @@ -232,12 +233,14 @@ def test_ForkingUDPServer(self): | |||
| self.dgram_examine) | |||
|
|
|||
| @requires_unix_sockets | |||
| @unittest.skipIf(sys.platform == 'win32', "no datagram support") | |||
There was a problem hiding this comment.
I'm seeing a lot of these - is there any more specific way to detect datagram support? The socket module already has a number of conditional constants, so maybe one of those will help?
|
@paulmon, please address the code reviews and resolve the merge conflict. Thank you! |
|
I don't currently have time to follow up on this PR or the immediate need for AF_UNIX in Windows |
|
Will python support AF_UNIX on windows ? |
These changes are incomplete, but this seems like a good point to get feedback on what I've learned so far.
Tests in test_asyncio are failing because create_unix_server and create_unix_connection are not implemented for Windows on the ProactorEventLoop class.
Linux implements create_unix_server and create_unix_connection on the _UnixEventSelectorLoop class, and the implementation cannot be simply copied because Linux uses signals, which don't exist for Windows. Windows appears to uses IO completion ports to achieve the same results.
https://bugs.python.org/issue33408