[issue11657] multiprocessing_{send,recv}fd fail with fds 256
Antoine Pitrou pit...@free.fr added the comment: For 3.3, it may be relevant that send/recvmsg are now available via the socket API (see #6560) I think sendfds/recvfds helper methods could be added to the socket type, so that programmers don't have to get the incantations right by themselves (note the plural, because passing several fds at once is more generic :-)). That said, +1 on committing Petri's patch. Will do it in a hour or two if noone is faster. -- nosy: +pitrou ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11657 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11657] multiprocessing_{send,recv}fd fail with fds 256
Changes by STINNER Victor victor.stin...@haypocalc.com: -- nosy: +haypo ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11657 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11657] multiprocessing_{send,recv}fd fail with fds 256
Antoine Pitrou pit...@free.fr added the comment: Now that I think of it, it would be nice to add some simple tests for recvfd and sendfd in test_multiprocessing. (also, when os.dup2() is available, you can easily generate an arbitrary big file descriptor) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11657 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11657] multiprocessing_{send,recv}fd fail with fds 256
Roundup Robot devn...@psf.upfronthosting.co.za added the comment: New changeset 5b2f357989bb by Antoine Pitrou in branch '3.2': Issue #11657: Fix sending file descriptors over 255 over a multiprocessing Pipe. http://hg.python.org/cpython/rev/5b2f357989bb New changeset 4e7a4e098f38 by Antoine Pitrou in branch 'default': Issue #11657: Fix sending file descriptors over 255 over a multiprocessing Pipe. http://hg.python.org/cpython/rev/4e7a4e098f38 -- nosy: +python-dev ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11657 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11657] multiprocessing_{send,recv}fd fail with fds 256
Roundup Robot devn...@psf.upfronthosting.co.za added the comment: New changeset d4d9a3e71897 by Antoine Pitrou in branch '2.7': Issue #11657: Fix sending file descriptors over 255 over a multiprocessing Pipe. http://hg.python.org/cpython/rev/d4d9a3e71897 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11657 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11657] multiprocessing_{send,recv}fd fail with fds 256
Antoine Pitrou pit...@free.fr added the comment: I committed a patch with some tests. Thank you! -- resolution: - fixed stage: patch review - committed/rejected status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11657 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11657] multiprocessing_{send,recv}fd fail with fds 256
Nick Coghlan ncogh...@gmail.com added the comment: For 3.3, it may be relevant that send/recvmsg are now available via the socket API (see #6560) -- nosy: +ncoghlan ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11657 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11657] multiprocessing_{send,recv}fd fail with fds 256
Charles-François Natali neolo...@free.fr added the comment: I'm fine if you fix it, as I'm currently really short on time myself. OK, I'll go ahead. For 3.3, it may be relevant that send/recvmsg are now available via the socket API (see #6560). Indeed. We might still need C code for the Windows part (I won't be able to help much with Windows, I'm blissfully ignorant). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11657 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11657] multiprocessing_{send,recv}fd fail with fds 256
Charles-François Natali neolo...@free.fr added the comment:
I looked at multiprocessing code, but didn't understand how to trigger a
call to these functions. Makes it hard to come up with a unit test...
Here's a sample test:
import _multiprocessing
import os
import socket
for i in range(4, 256):
os.dup2(1, i)
s, r = socket.socketpair()
pid = os.fork()
if pid == 0:
# child
fd = _multiprocessing.recvfd(r.fileno())
f = os.fdopen(fd)
print(f.read())
f.close()
else:
# parent
f = open('/etc/fstab')
_multiprocessing.sendfd(s.fileno(), f.fileno())
f.close()
os.waitpid(pid, 0)
What happens is that the parent process opens /etc/fstab, and sends the FD to
the child process, which prints it.
Now, if I run it with the current code, here's what I get:
cf@neobox:~/cpython$ ./python ~/test.py
Traceback (most recent call last):
File /home/cf/test.py, line 18, in module
_multiprocessing.sendfd(s.fileno(), f.fileno())
OSError: [Errno 9] Bad file descriptor
cf@neobox:~/cpython$ strace -e sendmsg ./python ~/test.py
sendmsg(3, {msg_name(0)=NULL, msg_iov(1)=[{\10, 1}], msg_controllen=16,
{cmsg_len=16, cmsg_level=SOL_SOCKET, cmsg_type=SCM_RIGHTS, {171137285}},
msg_flags=0}, 0) = -1 EBADF (Bad file descriptor)
Traceback (most recent call last):
File /home/cf/test.py, line 18, in module
_multiprocessing.sendfd(s.fileno(), f.fileno())
OSError: [Errno 9] Bad file descriptor
Duh, it's failing with EBADF.
Why?
cmsg-cmsg_len = CMSG_LEN(sizeof(int));
msg.msg_controllen = cmsg-cmsg_len;
*CMSG_DATA(cmsg) = fd;
Since we only set one byte in CMSG_DATA, if the other bytes are non-zero, the
value stored in CMSG_DATA(cmsg) ends up referring to a non existing FD, hence
the EBDAF.
With this simple patch:
diff -r e49dcb95241f Modules/_multiprocessing/multiprocessing.c
--- a/Modules/_multiprocessing/multiprocessing.cSun Aug 21 12:54:06
2011 +0200
+++ b/Modules/_multiprocessing/multiprocessing.cSun Aug 21 16:56:01
2011 +0200
@@ -111,7 +111,7 @@
cmsg-cmsg_type = SCM_RIGHTS;
cmsg-cmsg_len = CMSG_LEN(sizeof(int));
msg.msg_controllen = cmsg-cmsg_len;
-*CMSG_DATA(cmsg) = fd;
+*(int *)CMSG_DATA(cmsg) = fd;
Py_BEGIN_ALLOW_THREADS
res = sendmsg(conn, msg, 0);
@@ -154,7 +154,7 @@
if (res 0)
return PyErr_SetFromErrno(PyExc_OSError);
-fd = *CMSG_DATA(cmsg);
+fd = *(int *)CMSG_DATA(cmsg);
return Py_BuildValue(i, fd);
}
It works fine.
Note that if you want to check that for FD 255, you'd have to add something
like this at the top:
for i in range(4, 256):
os.dup2(1, i)
Note that I just used a cast to (int *) instead of memcpy() because CMSG_DATA
is actually int-aligned, so there's no risk of unaligned-access, and also it's
what's commonly used in the litterature.
So, would you like to add a test along those lines to test_multiprocessing?
AFAICT, multiprocessing.connection is not even documented, but this shows that
it really needs some testing...
--
nosy: +neologix
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue11657
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11657] multiprocessing_{send,recv}fd fail with fds 256
Jesse Noller jnol...@gmail.com added the comment: Yes, Charles - the test is not only welcome, but needed - it just can't rely on reading /etc/fstab ;) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11657 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11657] multiprocessing_{send,recv}fd fail with fds 256
Jesse Noller jnol...@gmail.com added the comment: Charles; you have +commit, it seems. I would welcome the patch and test (just as long as the aforementioned reliance on /etc/fstab was removed). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11657 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11657] multiprocessing_{send,recv}fd fail with fds 256
Charles-François Natali neolo...@free.fr added the comment: Charles; you have +commit, it seems. Yeah, I could of course do this myself, but since Petri already submitted a patch and seemed to be willing to update it with a test, I thought he might be interested in this (I've also seen he's participating to core-mentorship, so this would be a opportunity to help fixing a bug). As for the reliance on /etc/fstab, it was of course just a standalone example to demonstrate how one could test this corner-case ;-) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11657 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11657] multiprocessing_{send,recv}fd fail with fds 256
Petri Lehtinen pe...@digip.org added the comment: Charles-François Natali wrote: Yeah, I could of course do this myself, but since Petri already submitted a patch and seemed to be willing to update it with a test, I thought he might be interested in this (I've also seen he's participating to core-mentorship, so this would be a opportunity to help fixing a bug). I'm fine if you fix it, as I'm currently really short on time myself. When I was skimming through the multiprocessing tests a month ago, I noticed that much of the multiprocessing test suite is disabled, and it confused me. That was one reason why I didn't write a test right away. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11657 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11657] multiprocessing_{send,recv}fd fail with fds 256
Petri Lehtinen pe...@digip.org added the comment: I looked at multiprocessing code, but didn't understand how to trigger a call to these functions. Makes it hard to come up with a unit test... Ben: Do you still remember how you stumbled upon this issue? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11657 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11657] multiprocessing_{send,recv}fd fail with fds 256
Ben Darnell ben.darn...@gmail.com added the comment: These functions are used when passing a socket object across a multiprocessing.Queue. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11657 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11657] multiprocessing_{send,recv}fd fail with fds 256
Petri Lehtinen pe...@digip.org added the comment: Attached a patch for v2.7. It changes the assignments to memcpy() calls. -- keywords: +easy, needs review, patch nosy: +petri.lehtinen stage: - patch review versions: -Python 3.1 Added file: http://bugs.python.org/file22786/issue11657.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11657 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11657] multiprocessing_{send,recv}fd fail with fds 256
Jean-Paul Calderone invalid@example.invalid added the comment: Thanks for the patch Petri. Are you interested in writing a unit test for this as well? -- nosy: +exarkun ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11657 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11657] multiprocessing_{send,recv}fd fail with fds 256
Changes by Antoine Pitrou pit...@free.fr: -- nosy: +asksol, jnoller versions: +Python 2.7, Python 3.1, Python 3.2, Python 3.3 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11657 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11657] multiprocessing_{send,recv}fd fail with fds 256
New submission from Ben Darnell ben.darn...@gmail.com:
Line 125 of multiprocessing.c is *CMSG_DATA(cmsg) = fd;. CMSG_DATA returns
an unsigned char*, while fd is an int, so this code does not support file
descriptors 256 (additionally, I'm not sure if the buffer is guaranteed to be
initialized with zeros). recvfd has an analogous problem at line 168. Both
of these need to be changed to copy the entire integer, e.g. by casting the
result of CMSG_DATA to an int*.
http://hg.python.org/cpython/file/5deb2094f033/Modules/_multiprocessing/multiprocessing.c
--
messages: 131947
nosy: Ben.Darnell
priority: normal
severity: normal
status: open
title: multiprocessing_{send,recv}fd fail with fds 256
type: behavior
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue11657
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
