Message142627
> 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... |
|
Date |
User |
Action |
Args |
2011-08-21 15:07:53 | neologix | set | recipients:
+ neologix, exarkun, jnoller, asksol, Ben.Darnell, petri.lehtinen |
2011-08-21 15:07:53 | neologix | set | messageid: <1313939273.2.0.395402345028.issue11657@psf.upfronthosting.co.za> |
2011-08-21 15:07:52 | neologix | link | issue11657 messages |
2011-08-21 15:07:51 | neologix | create | |
|