Charles-François Natali <[email protected]> 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.c Sun Aug 21 12:54:06
2011 +0200
+++ b/Modules/_multiprocessing/multiprocessing.c Sun 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 <[email protected]>
<http://bugs.python.org/issue11657>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
