-
-
Notifications
You must be signed in to change notification settings - Fork 178
Detach socket in create_server, create_connection, and other APIs #449
base: master
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -87,6 +87,12 @@ def _set_reuseport(sock): | |
'SO_REUSEPORT defined but not implemented.') | ||
|
||
|
||
def _copy_and_detach_socket(sock): | ||
new_sock = socket.socket(sock.family, sock.type, sock.proto, sock.fileno()) | ||
sock.detach() | ||
return new_sock | ||
|
||
|
||
# Linux's sock.type is a bitmask that can include extra info about socket. | ||
_SOCKET_TYPE_MASK = 0 | ||
if hasattr(socket, 'SOCK_NONBLOCK'): | ||
|
@@ -768,9 +774,11 @@ def create_connection(self, protocol_factory, host=None, port=None, *, | |
raise OSError('Multiple exceptions: {}'.format( | ||
', '.join(str(exc) for exc in exceptions))) | ||
|
||
elif sock is None: | ||
raise ValueError( | ||
'host and port was not specified and no sock specified') | ||
else: | ||
if sock is None: | ||
raise ValueError( | ||
'host and port was not specified and no sock specified') | ||
sock = _copy_and_detach_socket(sock) | ||
|
||
transport, protocol = yield from self._create_connection_transport( | ||
sock, protocol_factory, ssl, server_hostname) | ||
|
@@ -827,6 +835,7 @@ def create_datagram_endpoint(self, protocol_factory, | |
raise ValueError( | ||
'socket modifier keyword arguments can not be used ' | ||
'when sock is specified. ({})'.format(problems)) | ||
sock = _copy_and_detach_socket(sock) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Who closes the new cloned socket in case of failure at this point? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I moved this line after the |
||
sock.setblocking(False) | ||
r_addr = None | ||
else: | ||
|
@@ -1024,6 +1033,7 @@ def create_server(self, protocol_factory, host=None, port=None, | |
else: | ||
if sock is None: | ||
raise ValueError('Neither host/port nor sock were specified') | ||
sock = _copy_and_detach_socket(sock) | ||
sockets = [sock] | ||
|
||
server = Server(self, sockets) | ||
|
@@ -1045,6 +1055,7 @@ def connect_accepted_socket(self, protocol_factory, sock, *, ssl=None): | |
This method is a coroutine. When completed, the coroutine | ||
returns a (transport, protocol) pair. | ||
""" | ||
sock = _copy_and_detach_socket(sock) | ||
transport, protocol = yield from self._create_connection_transport( | ||
sock, protocol_factory, ssl, '', server_side=True) | ||
if self._debug: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1240,11 +1240,12 @@ def connection_made(self, transport): | |
sock_ob = socket.socket(type=socket.SOCK_STREAM) | ||
sock_ob.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) | ||
sock_ob.bind(('0.0.0.0', 0)) | ||
sock_ob_fd = sock_ob.fileno() | ||
|
||
f = self.loop.create_server(TestMyProto, sock=sock_ob) | ||
server = self.loop.run_until_complete(f) | ||
sock = server.sockets[0] | ||
self.assertIs(sock, sock_ob) | ||
self.assertEqual(sock.fileno(), sock_ob_fd) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should check that sock_ob is detach: sock_ob.fileno() must be equal to -1. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed |
||
|
||
host, port = sock.getsockname() | ||
self.assertEqual(host, '0.0.0.0') | ||
|
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.
You should use detach() to get the FD.
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.
Done