-
-
Notifications
You must be signed in to change notification settings - Fork 32k
bpo-28724: Add methods send_fds, recv_fds to the socket module #12889
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
Changes from all commits
fd7b901
2558c5d
dfdeb76
bc58424
cc6973c
5050e59
fe8ff1a
e98918f
c3fcd6f
d497c15
035845a
e6d22e0
f84d6db
07b05dc
8b96493
7c46e7a
2c32da1
75bffbc
ba29ca2
83e422d
910461b
b134b48
ee950f8
36e157d
0f03c42
bf67bd8
1ac7fbf
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 |
---|---|---|
|
@@ -12,6 +12,8 @@ | |
socket() -- create a new socket object | ||
socketpair() -- create a pair of new socket objects [*] | ||
fromfd() -- create a socket object from an open file descriptor [*] | ||
send_fds() -- Send file descriptor to the socket. | ||
recv_fds() -- Recieve file descriptors from the socket. | ||
fromshare() -- create a socket object from data received from socket.share() [*] | ||
gethostname() -- return the current hostname | ||
gethostbyname() -- map a hostname to its IP number | ||
|
@@ -463,6 +465,40 @@ def fromfd(fd, family, type, proto=0): | |
nfd = dup(fd) | ||
return socket(family, type, proto, nfd) | ||
|
||
if hasattr(_socket.socket, "sendmsg"): | ||
import array | ||
|
||
def send_fds(sock, buffers, fds, flags=0, address=None): | ||
""" send_fds(sock, buffers, fds[, flags[, address]]) -> integer | ||
|
||
Send the list of file descriptors fds over an AF_UNIX socket. | ||
""" | ||
return sock.sendmsg(buffers, [(_socket.SOL_SOCKET, | ||
_socket.SCM_RIGHTS, array.array("i", fds))]) | ||
__all__.append("send_fds") | ||
|
||
if hasattr(_socket.socket, "recvmsg"): | ||
import array | ||
|
||
def recv_fds(sock, bufsize, maxfds, flags=0): | ||
""" recv_fds(sock, bufsize, maxfds[, flags]) -> (data, list of file | ||
descriptors, msg_flags, address) | ||
|
||
Receive up to maxfds file descriptors returning the message | ||
data and a list containing the descriptors. | ||
""" | ||
# Array of ints | ||
fds = array.array("i") | ||
msg, ancdata, flags, addr = sock.recvmsg(bufsize, | ||
_socket.CMSG_LEN(maxfds * fds.itemsize)) | ||
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. This will likely fail on FreeBSD as RFC 3542 requires portable applications to use CMSG_SPACE if I am not mistaken. 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. We should check that this works on FreeBSD to avoid broken buildbots with obscure errors. 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. @pablogsal You mean at test level ? 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 mean that FreeBSD systems running that code can fail and that will likely appear when running the tests that go over this code path. This is an example of similar scenarios of CMSG_LEN vs CMSG_SPACE: I could be wrong with my argument, that's why I think we should check directly on a FreeBSD system. 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 dont have a freeBSD system to confirm this atm. When I do find, I will confirm but someone else can help confirm this. Do you have a FreeBSD to help check this? 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. Here are the custom builds: https://buildbot.python.org/all/#/builders/175/builds/39 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. It seems that my suspicions were correct:
but it could be something also on top of CMSG_LEN 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. @koobs Could you please take a look? The new feature is failing on FreeBSD. 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 can confirm that is the same error as in OSX so it may be easier to investigate there. 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. According to the FreeBSD/OSX man page:
I am not sure if they mean that is done automatically or that you should do that. |
||
for cmsg_level, cmsg_type, cmsg_data in ancdata: | ||
if (cmsg_level == _socket.SOL_SOCKET and cmsg_type == _socket.SCM_RIGHTS): | ||
fds.frombytes(cmsg_data[: | ||
len(cmsg_data) - (len(cmsg_data) % fds.itemsize)]) | ||
vstinner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
return msg, list(fds), flags, addr | ||
vstinner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
__all__.append("recv_fds") | ||
|
||
if hasattr(_socket.socket, "share"): | ||
def fromshare(info): | ||
""" fromshare(info) -> socket object | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
The socket module now has the :func:`socket.send_fds` and :func:`socket.recv.fds` methods. | ||
Contributed by Joannah Nanjekye, Shinya Okano and Victor Stinner. |
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 document at least what is the fds parameter. I suggest to document it as sequence of file descriptors. Is it possible to send 0 file descriptor?
What is the return value?
Does someone know if it's possible that the call send only a few file descriptors, but not all of them? What if it's called with 100 000 file descriptors for example?
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.
The return type for
sendmsg()
is int AFAIK. Am not sure about these other edge cases but I can look.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.
I tested to send 1000 file descriptors: sendmsg() fails with "OSError: [Errno 22] Invalid argument".
For partial send, it can happen, as in socket.send(). In that case, according to https://stackoverflow.com/questions/38431713/partial-read-write-issue-in-sendmsg-recvmsg the file descriptors are already sent and should be be send again, socket.sendmsg() should be called again, but only with buffers (adjusted to the offset equal to the send_fds() result).
In short, you don't have to do anything. I'm not comfortable to document how to handle partial send since I'm not sure that the behavior is portable.
sendmsg() doesn't warn about partial send. Maybe we should copy send() warning into sendmsg() and send_fds(): "Applications are responsible for checking that all data has been sent; if only some of the data was transmitted, the application needs to attempt delivery of the remaining data."