Issue 4892: Sending Connection-objects over multiprocessing connections fails (original) (raw)

Issue4892

process

Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Jimbofbx, asksol, dragonfyre13, dsvensson, gsson, jnoller, jodal, kristjan.jonsson, pitrou, python-dev, sbt
Priority: low Keywords: patch

Created on 2009-01-09 11:46 by gsson, last changed 2022-04-11 14:56 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
mp_pickle_conn.patch sbt,2012-04-09 11:49 review
mp_pickle_conn.patch sbt,2012-04-10 13:15 review
mp_pickle_conn.patch sbt,2012-04-11 18:22 review
mp_pickle_conn.patch sbt,2012-04-19 09:44 review
mp_pickle_conn.patch sbt,2012-04-19 10:57 review
Messages (31)
msg79464 - (view) Author: Henrik Gustafsson (gsson) Date: 2009-01-09 11:46
It seems the old pyprocessing (http://pyprocessing.berlios.de/) can do some things that the new multiprocessing package can not; sending/receiving connection objects for one. This is a quite handy functionality, so it would be nice if it were reintroduced. Also, the error message isn't very helpful. Failing test below. $ python2.6 pipetest2.py asdf Traceback (most recent call last): File "a.py", line 10, in print c1.recv() TypeError: Required argument 'handle' (pos 1) not found $ PYTHONPATH=processing-0.52-py2.5-macosx-10.5-i386.egg python2.5 pipetest2.py asdf Connection(handle=5) $ PYTHONPATH=multiprocessing-2.6.0.2-py2.5-macosx-10.5-i386.egg python2.5 pipetest2.py asdf Traceback (most recent call last): File "pipetest2.py", line 10, in print c1.recv() TypeError: function takes at least 1 argument (0 given) $ uname -a Darwin midori.local 9.6.0 Darwin Kernel Version 9.6.0: Mon Nov 24 17:37:00 PST 2008; root:xnu-1228.9.59~1/RELEASE_I386 i386
msg79465 - (view) Author: Henrik Gustafsson (gsson) Date: 2009-01-09 11:47
$ cat pipetest2.py try: from multiprocessing import Pipe except ImportError: from processing import Pipe c1, c2 = Pipe(duplex=False) c2.send('asdf') print c1.recv() c2.send(c1) print c1.recv()
msg79474 - (view) Author: Jesse Noller (jnoller) * (Python committer) Date: 2009-01-09 15:45
thanks for filing this. I'll need to compare the two code bases and figure out why it's either regressed, or Richard removed it prior to the integration.
msg84707 - (view) Author: Jesse Noller (jnoller) * (Python committer) Date: 2009-03-30 23:10
Before I can logically support this, I need a clear use case that supports the idea that this should be supported in the current version of multiprocessing.
msg91964 - (view) Author: Daniel Svensson (dsvensson) Date: 2009-08-26 09:54
A typical use case would be for a server to receive a connection, and then send that connection over to another process that does the actual work. This used to work with pyprocessing, and the support seems to be available in multiprocessing.c -> multiprocessing_sendfd, but not used.
msg91965 - (view) Author: Daniel Svensson (dsvensson) Date: 2009-08-26 10:51
And to be clear, I have enabled connection pickling by issuing: multiprocessing.allow_connection_pickling()
msg91968 - (view) Author: Daniel Svensson (dsvensson) Date: 2009-08-26 12:30
When reverting this commit stuff works: http://svn.python.org/view/python/trunk/Lib/multiprocessing/reduction.py?r1=64257&r2=65016
msg91969 - (view) Author: Daniel Svensson (dsvensson) Date: 2009-08-26 12:33
Ehm.. completly broken url in prev message.. Revision 65016, "Apply Amaury's patch to multiprocessing for issue 3125, removes the copy_reg and replaces it with ForkingPickler.register(), which should resolve the conflict with the global registry/ctypes" is what I'm refering to. Without this patch, the reducers/rebuilders are properly registered in the pickler that connection.h (srsly, code in header files?) later uses.
msg126447 - (view) Author: Tim Alexander (dragonfyre13) Date: 2011-01-18 02:21
Wanted to quickly comment here, as I'm dealing with this issue as well, that I did find a workaround for avoiding it as far back as 2.6 (and it's not "don't pass a Pipe through a Pipe") multiprocessing.reduction can already do this, though I don't entirely know why this isn't automatically done if it's a connection object. >>> from multiprocessing import Pipe, reduction >>> i, o = Pipe() >>> reduced = reduction.reduce_connection(i) >>> newi = reduced[0](*reduced[1]) >>> newi.send("hi") >>> o.recv() 'hi' >>> The reduced[0](*reduced[1]) line is actually calling reduction.rebuild_connection, as that function is the first element in the tuple, and the second element is the arguments to be passed to it. I can't seem to find any info on reduction.reduce_connection, so I don't know if this is how this was intended to be handled or not. P.S. Tested on Win (XP) and Linux (Ubuntu 10.10), so there's no weird windows socket stuff that should go wrong with this.
msg157704 - (view) Author: James Hutchison (Jimbofbx) Date: 2012-04-06 22:31
err, is it possible to edit out those file paths? I didn't intend them to be in the message. I'd appreciate it if someone with the privileges to do so could remove them.
msg157724 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-04-07 10:40
> err, is it possible to edit out those file paths? I don't know how to do that. If you want I can remove the message altogether. But I don't see anything confidential or exploitable in your message.
msg157736 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2012-04-07 15:52
Jimbofbx wrote: > def main(): > from multiprocessing import Pipe, reduction > i, o = Pipe() > print(i); > reduced = reduction.reduce_connection(i) > print(reduced); > newi = reduced[0](*reduced[1]) > print(newi); > newi.send("hi") > o.recv() On Windows with a PipeConnection object you should use rebuild_pipe_connection() instead of rebuild_connection(). With that change, on Python 3.3 I get <multiprocessing.connection.PipeConnection object at 0x025BBCB0> (<function rebuild_pipe_connection at 0x0262F420>, (('\\\\.\\pipe\\pyc-6000-1-30lq4p', 356, False), True, True)) <multiprocessing.connection.PipeConnection object at 0x029FF710> Having said all that I agree multiprocessing.reduction should be fixed. Maybe an enable_pickling_support() function could be added to register the necessary things with copyreg.
msg157737 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-04-07 16:14
> Having said all that I agree multiprocessing.reduction should be > fixed. Maybe an enable_pickling_support() function could be added to > register the necessary things with copyreg. Why not simply use ForkingPickler?
msg157739 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2012-04-07 17:26
ForkingPickler is only used when creating a child process. The multiprocessing.reduction module is only really intended for sending stuff to *pre-existing* processes. As things stand, after importing multiprocessing.reduction you can do something like buf = io.BytesIO() pickler = ForkingPickler(buf) pickler.dump(conn) data = buf.getvalue() writer.send_bytes(data) But that is rather less simple and obvious than just doing writer.send(conn) which was possible in pyprocessing. Originally just importing the module magically registered the reduce functions with copyreg. Since this was undesirable, the reduction functions were instead registered with ForkingPickler. But this fix rather missed the point of the module.
msg157740 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-04-07 17:28
> ForkingPickler is only used when creating a child process. The > multiprocessing.reduction module is only really intended for sending > stuff to *pre-existing* processes. But ForkingPickler could be used in multiprocessing.connection, couldn't it?
msg157743 - (view) Author: James Hutchison (Jimbofbx) Date: 2012-04-07 17:46
@pitrou You can just delete my original post. I'll repost an edited version here for reference original post with paths removed: This is an issue for me (Python 3.2). I have a custom pool that sends arguments for a function call over a pipe. I cannot send another pipe as an argument. Tim's workaround also does not work for me (win xp 32bit and 64bit) From what I can tell, you can only send a connection as a direct argument to a function call. This limits what I can do because I cannot introduce new pipes to a worker process after it is instantiated. Using this code: def main(): from multiprocessing import Pipe, reduction i, o = Pipe() print(i); reduced = reduction.reduce_connection(i) print(reduced); newi = reduced[0](*reduced[1]) print(newi); newi.send("hi") o.recv() if __name__ == "__main__": main(); This is my output: <read-write PipeConnection, handle 1760> (<function rebuild_connection at 0x00FD4C00>, (('\\\\.\\pipe\\pyc-3156-1-q5wwnr', 1756, False), True, True)) <read-write Connection, handle 1720> >>> newi.send("hi") IOError: [Errno 10038] An operation was attempted on something that is not a socket As you can see, the handle changes
msg157745 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2012-04-07 18:21
> But ForkingPickler could be used in multiprocessing.connection, > couldn't it? I suppose so. Note that the way a connection handle is transferred between existing processes is unnecessarily inefficient on Windows. A background server thread (one per process) has to be started and the receiving process must connect back to the sending process to receive its duplicate handle. There is a simpler way to do this on Windows. The sending process duplicates the handle, and the receiving process duplicates that second handle using DuplicateHandle() and the DUPLICATE_CLOSE_SOURCE flag. That way no server thread is necessary on Windows. I got this to work recently for pickling references to file handles for mmaps on. (A server thread would still be necessary on Unix.)
msg157749 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2012-04-07 19:08
> There is a simpler way to do this on Windows. The sending process > duplicates the handle, and the receiving process duplicates that second > handle using DuplicateHandle() and the DUPLICATE_CLOSE_SOURCE flag. That > way no server thread is necessary on Windows. Note that this should not be done for socket handles since DuplicateHandle() is not supposed to work for them. socket.share() and socket.fromshare() with a server thread can be used for sockets.
msg157753 - (view) Author: James Hutchison (Jimbofbx) Date: 2012-04-07 19:35
Shouldn't reduce_pipe_connection just be an alias for reduce_connection in unix so that using reduce_pipe_connection would work for both win and unix? My understanding after looking at the code is that reduce_pipe_connection isn't defined for non-win32, although I haven't tested it to see if that's true. Of course, ideally a pipe connection would just pickle and unpickle properly out-of-the-box, which I think was the original intent. Here's a complete, working example with Python 3.2 tested on Win 7 64-bit: import sys from multiprocessing import Process,Pipe, reduction def main(): print("starting"); i, o = Pipe(False) parent, child = Pipe(); reducedchild = reduce_pipe(child); p = Process(target=helper, args=(i,)); p.start(); parent.send("hi"); o.send(reducedchild); print(parent.recv()); print("finishing"); p.join(); print("done"); def helper(inPipe): childPipe = expand_reduced_pipe(inPipe.recv()); childPipe.send("child got: " + childPipe.recv()); return; def reduce_pipe(pipe): if sys.platform == "win32": return reduction.reduce_pipe_connection(pipe); else: return reduction.reduce_connection(pipe); def expand_reduced_pipe(reduced_pipe): return reduced_pipe[0](*reduced_pipe[1]); if __name__ == "__main__": main();
msg157838 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2012-04-09 11:49
There is an undocumented function multiprocessing.allow_connection_pickling() whose docstring claims it allows connection and socket objects to be pickled. The attached patch fixes the multiprocessing.reduction module so that it works correctly. This means that TestPicklingConnections can be reenabled in the unit tests. The patch uses the new socket.share() and socket.fromshare() methods on Windows.
msg157840 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-04-09 12:04
Unless there's a technical barrier, I still think it would be better to use ForkingPickler in multiprocessing.connection, rather than modify global state (copyreg). The pickling support is multiprocessing-specific and wouldn't make sense for other pickles (e.g. stored to disk).
msg157852 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2012-04-09 15:44
I just want to point out that each time socket.share() is called, the resulting data can only be used once by socket.fromshare(). I'm mentioning this because I know there is some caching mechanism in reduction.py and that this data is not cacheable/reuseable.
msg157950 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2012-04-10 13:15
Updated patch which uses ForkingPickler in Connection.send(). Note that connection sharing still has to be enabled using allow_connection_pickling(). Support could be enabled automatically, but that would introduce more circular imports which confuse me. It might be worthwhile refactoring to eliminate all circular imports.
msg157960 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-04-10 15:32
> Support could be enabled automatically, but that would introduce more > circular imports which confuse me. Are you sure? AFAICT: - connection depends on forking - reduction depends on forking and connection But connection doesn't depend on reduction, neither does forking.
msg157963 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2012-04-10 16:11
> But connection doesn't depend on reduction, neither does forking. If registration of (Pipe)Connection is done in reduction then you can't make (Pipe)Connection picklable *automatically* unless you make connection depend on reduction (possibly indirectly). A circular import can be avoided by making reduction not import connection at module level. So not hard to fix.
msg158065 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2012-04-11 18:22
The last patch did not work on Unix. Here is a new version where the reduction functions are automatically registered, so allow_connection_pickling() is redundant.
msg158682 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-04-18 23:10
Could you regenerate your patch now that the win32 -> _winapi changes have been applied?
msg158711 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2012-04-19 09:44
Up to date patch.
msg158714 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2012-04-19 10:57
A couple of minor changes based on Antoine's earlier review (which I did not notice till now).
msg159207 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2012-04-24 20:58
New changeset 08d4c2fe51ea by Antoine Pitrou in branch 'default': Issue #4892: multiprocessing Connections can now be transferred over multiprocessing Connections. http://hg.python.org/cpython/rev/08d4c2fe51ea
msg159208 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-04-24 20:59
Thanks, Richard. I have now committed the patch. Hopefully the Windows buildbots will be ok :)
History
Date User Action Args
2022-04-11 14:56:43 admin set github: 49142
2012-04-24 20:59:07 pitrou set status: open -> closedresolution: fixedmessages: + stage: patch review -> resolved
2012-04-24 20:58:23 python-dev set nosy: + python-devmessages: +
2012-04-19 10:57:38 sbt set files: + mp_pickle_conn.patchmessages: +
2012-04-19 09:44:45 sbt set files: + mp_pickle_conn.patchmessages: +
2012-04-18 23:10:46 pitrou set stage: needs patch -> patch review
2012-04-18 23:10:40 pitrou set messages: +
2012-04-11 18:22:23 sbt set files: + mp_pickle_conn.patchmessages: +
2012-04-10 16:11:15 sbt set messages: +
2012-04-10 15:32:42 pitrou set messages: +
2012-04-10 13:15:29 sbt set files: + mp_pickle_conn.patchmessages: +
2012-04-09 15:44:10 kristjan.jonsson set messages: +
2012-04-09 13:52:10 kristjan.jonsson set nosy: + kristjan.jonsson
2012-04-09 12:04:29 pitrou set messages: +
2012-04-09 11:49:58 sbt set files: + mp_pickle_conn.patchkeywords: + patchmessages: +
2012-04-07 19:35:09 Jimbofbx set messages: +
2012-04-07 19:08:37 sbt set messages: +
2012-04-07 18:21:33 sbt set messages: +
2012-04-07 17:52:53 pitrou set messages: -
2012-04-07 17:46:10 Jimbofbx set messages: +
2012-04-07 17:28:46 pitrou set messages: +
2012-04-07 17:26:05 sbt set messages: +
2012-04-07 16:14:11 pitrou set messages: +
2012-04-07 15:52:05 sbt set messages: +
2012-04-07 10:40:42 pitrou set assignee: jnoller -> nosy: + sbtversions: + Python 3.3, - Python 3.2
2012-04-07 10:40:27 pitrou set nosy: + pitroumessages: +
2012-04-06 22:31:51 Jimbofbx set messages: +
2012-04-06 22:19:21 Jimbofbx set nosy: + Jimbofbxmessages: +
2011-01-18 02:21:11 dragonfyre13 set nosy: + dragonfyre13messages: +
2010-08-31 10:53:39 asksol set nosy: + asksol
2010-08-09 04:44:48 terry.reedy set versions: - Python 3.1, Python 2.7
2010-08-07 21:29:38 terry.reedy set stage: needs patchversions: + Python 3.1, Python 2.7, Python 3.2, - Python 2.6
2010-03-19 22:58:08 jodal set nosy: + jodal
2009-08-26 12:33:47 dsvensson set messages: +
2009-08-26 12:30:13 dsvensson set messages: +
2009-08-26 10:51:50 dsvensson set messages: +
2009-08-26 09:54:53 dsvensson set nosy: + dsvenssonmessages: +
2009-03-30 23:10:05 jnoller set priority: normal -> lowmessages: +
2009-03-30 22:50:39 ronaldoussoren set components: - macOS
2009-01-18 19:35:13 jnoller set priority: normaltype: behavior -> enhancement
2009-01-09 15:45:32 jnoller set messages: +
2009-01-09 15:45:00 jnoller set assignee: jnollernosy: + jnoller
2009-01-09 11:47:52 gsson set messages: +
2009-01-09 11:46:06 gsson create