This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author neologix
Recipients Ben.Darnell, asksol, exarkun, jnoller, neologix, petri.lehtinen
Date 2011-08-21.15:07:51
SpamBayes Score 1.4249713e-13
Marked as misclassified No
Message-id <1313939273.2.0.395402345028.issue11657@psf.upfronthosting.co.za>
In-reply-to
Content
> 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...
History
Date User Action Args
2011-08-21 15:07:53neologixsetrecipients: + neologix, exarkun, jnoller, asksol, Ben.Darnell, petri.lehtinen
2011-08-21 15:07:53neologixsetmessageid: <1313939273.2.0.395402345028.issue11657@psf.upfronthosting.co.za>
2011-08-21 15:07:52neologixlinkissue11657 messages
2011-08-21 15:07:51neologixcreate