classification
Title: test_multiprocessing causes test_ctypes to fail
Type: behavior Stage: patch review
Components: Tests Versions: Python 3.1, Python 2.7
process
Status: closed Resolution: out of date
Dependencies: Superseder:
Assigned To: jnoller Nosy List: BreamoreBoy, Rhamphoryncus, amaury.forgeotdarc, asksol, jnoller, roudkerk, theller
Priority: high Keywords: patch

Created on 2008-06-16 17:31 by theller, last changed 2010-09-09 11:32 by asksol. This issue is now closed.

Files
File name Uploaded Description Edit
sharedctypes.py.patch roudkerk, 2008-06-18 15:45
multiprocessing.patch theller, 2008-06-18 18:13
no_copyreg.patch amaury.forgeotdarc, 2008-06-20 00:04
py3k_no_copyreg.patch jnoller, 2008-07-16 19:21
Messages (29)
msg68276 - (view) Author: Thomas Heller (theller) * (Python committer) Date: 2008-06-16 17:31
test_ctypes, when run after testmultiprocessing, fails:

...
======================================================================
ERROR: test_simple (ctypes.test.test_pickling.PickleTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "c:\svn\trunk\lib\ctypes\test\test_pickling.py", line 29, in
test_simple
    dst = self.loads(self.dumps(src))
  File "c:\svn\trunk\lib\ctypes\test\test_pickling.py", line 19, in dumps
    return pickle.dumps(item)
  File "c:\svn\trunk\lib\pickle.py", line 1366, in dumps
    Pickler(file, protocol).dump(obj)
  File "c:\svn\trunk\lib\pickle.py", line 224, in dump
    self.save(obj)
  File "c:\svn\trunk\lib\pickle.py", line 301, in save
    rv = reduce(obj)
  File "c:\svn\trunk\lib\multiprocessing\sharedctypes.py", line 121, in
reduce_ctype
    assert_spawning(obj)
  File "c:\svn\trunk\lib\multiprocessing\forking.py", line 25, in
assert_spawning
    ' through inheritance' % type(self).__name__
RuntimeError: c_long objects should only be shared between processes
through inheritance

======================================================================
ERROR: test_simple (ctypes.test.test_pickling.PickleTest_1)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "c:\svn\trunk\lib\ctypes\test\test_pickling.py", line 29, in
test_simple
    dst = self.loads(self.dumps(src))
  File "c:\svn\trunk\lib\ctypes\test\test_pickling.py", line 71, in dumps
    return pickle.dumps(item, 1)
  File "c:\svn\trunk\lib\pickle.py", line 1366, in dumps
    Pickler(file, protocol).dump(obj)
  File "c:\svn\trunk\lib\pickle.py", line 224, in dump
    self.save(obj)
  File "c:\svn\trunk\lib\pickle.py", line 301, in save
    rv = reduce(obj)
  File "c:\svn\trunk\lib\multiprocessing\sharedctypes.py", line 121, in
reduce_ctype
    assert_spawning(obj)
  File "c:\svn\trunk\lib\multiprocessing\forking.py", line 25, in
assert_spawning
    ' through inheritance' % type(self).__name__
RuntimeError: c_long objects should only be shared between processes
through inheritance

======================================================================
ERROR: test_simple (ctypes.test.test_pickling.PickleTest_2)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "c:\svn\trunk\lib\ctypes\test\test_pickling.py", line 29, in
test_simple
    dst = self.loads(self.dumps(src))
  File "c:\svn\trunk\lib\ctypes\test\test_pickling.py", line 75, in dumps
    return pickle.dumps(item, 2)
  File "c:\svn\trunk\lib\pickle.py", line 1366, in dumps
    Pickler(file, protocol).dump(obj)
  File "c:\svn\trunk\lib\pickle.py", line 224, in dump
    self.save(obj)
  File "c:\svn\trunk\lib\pickle.py", line 301, in save
    rv = reduce(obj)
  File "c:\svn\trunk\lib\multiprocessing\sharedctypes.py", line 121, in
reduce_ctype
    assert_spawning(obj)
  File "c:\svn\trunk\lib\multiprocessing\forking.py", line 25, in
assert_spawning
    ' through inheritance' % type(self).__name__
RuntimeError: c_long objects should only be shared between processes
through inheritance

----------------------------------------------------------------------
msg68278 - (view) Author: Jesse Noller (jnoller) * (Python committer) Date: 2008-06-16 17:53
Adam, is this due to the c-code tainting you pointed out earlier?
msg68279 - (view) Author: Thomas Heller (theller) * (Python committer) Date: 2008-06-16 18:01
IMO this problem occurs because multiprocessing registers pickling
support for ctypes, but the ctypes in trunk already has support for
pickling.
msg68280 - (view) Author: Adam Olsen (Rhamphoryncus) Date: 2008-06-16 18:12
Jesse, can you be more specific?

Thomas, do you have a specific command to reproduce this?  It runs fine
if I do "./python -m test.regrtest -v test_multiprocessing test_ctypes".
 That's with amaury's patch from 3100 applied.
msg68282 - (view) Author: Jesse Noller (jnoller) * (Python committer) Date: 2008-06-16 18:26
Sorry Adam, I was referencing:

http://bugs.python.org/issue3102

Also, it is possible issue: http://bugs.python.org/issue3102 is related?
msg68283 - (view) Author: Jesse Noller (jnoller) * (Python committer) Date: 2008-06-16 18:28
Sorry, the first link should have been:
http://bugs.python.org/issue3093
msg68284 - (view) Author: Thomas Heller (theller) * (Python committer) Date: 2008-06-16 18:37
Adam Olsen schrieb:
> Thomas, do you have a specific command to reproduce this?  It runs fine
> if I do "./python -m test.regrtest -v test_multiprocessing test_ctypes".
>  That's with amaury's patch from 3100 applied.

It seems the failure only occurs on Windows.  On linux it does work for me too.

See the traceback that I posted, and this: http://www.python.org/dev/buildbot/trunk/x86%20XP-3%20trunk/builds/1606/step-test/0
msg68285 - (view) Author: Adam Olsen (Rhamphoryncus) Date: 2008-06-16 18:45
I see no common symbols between #3102 and #3092, so unless I missed
something, they shouldn't be involved.

I second the notion that multiprocessing's use of pickle is the
triggering factor.  Registering so many types is ugly, and IMO it
shouldn't register anything it doesn't control.  We should either
register them global or not at all, and *never* as a side-effect of
loading a separate module.

I do see some win32-specific behaviour, which may be broken.  Thomas,
wanna try commenting out these two lines in sharedtypes.py:rebuild_ctype?

    if sys.platform == 'win32' and type_ not in copy_reg.dispatch_table:
        copy_reg.pickle(type_, reduce_ctype)
msg68287 - (view) Author: Thomas Heller (theller) * (Python committer) Date: 2008-06-16 18:51
> Adam Olsen <rhamph@gmail.com> added the comment:
> 
> I see no common symbols between #3102 and #3092, so unless I missed
> something, they shouldn't be involved.
> 
> I second the notion that multiprocessing's use of pickle is the
> triggering factor.  Registering so many types is ugly, and IMO it
> shouldn't register anything it doesn't control.  We should either
> register them global or not at all, and *never* as a side-effect of
> loading a separate module.
> 
> I do see some win32-specific behaviour, which may be broken.  Thomas,
> wanna try commenting out these two lines in sharedtypes.py:rebuild_ctype?
> 
>     if sys.platform == 'win32' and type_ not in copy_reg.dispatch_table:
>         copy_reg.pickle(type_, reduce_ctype)

This fixes the failure in test_ctypes, but test_multiprocessing no longer works:

c:\svn\trunk\PCbuild>.\\python_d  -E -tt ../lib/test/regrtest.py test_multiprocessing test_ctypes
test_multiprocessing
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "c:\svn\trunk\lib\multiprocessing\forking.py", line 297, in main
self = load(from_parent)
EOFError
[48274 refs]
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "c:\svn\trunk\lib\multiprocessing\forking.py", line 297, in main
self = load(from_parent)
EOFError
[48763 refs]
test test_multiprocessing failed -- errors occurred; run in verbose mode for details
test_ctypes
1 test OK.
1 test failed:
    test_multiprocessing
[118894 refs]

c:\svn\trunk\PCbuild>
msg68365 - (view) Author: roudkerk (roudkerk) Date: 2008-06-18 15:45
This patch to sharedctypes should fix the problem by adding a
__reduce_ex__() method to a shared ctype object instead of using copy_reg.
msg68367 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-06-18 16:10
But why this is win32 specific?

Is it because windows cannot fork(), so data has to be copied through
the pickle mechanism?
In this case let's remove the "if win32" statement, and always execute
the body.
msg68368 - (view) Author: roudkerk (roudkerk) Date: 2008-06-18 17:37
> But why this is win32 specific?
> 
> Is it because windows cannot fork(), so data has to be copied through
> the pickle mechanism?
> In this case let's remove the "if win32" statement, and always execute
> the body.

Yes, on Windows pickling is needed to pass data to a child process.  In
other contexts these objects are NOT picklable because you would have to
worry about garbage collection of the original object before the copy is
rebuilt by the other process.  On unix pickling will always fail even if
it "if win32" statement was removed.
msg68369 - (view) Author: Thomas Heller (theller) * (Python committer) Date: 2008-06-18 18:13
roudkerk schrieb:
> roudkerk <r.m.oudkerk@gmail.com> added the comment:
> 
> This patch to sharedctypes should fix the problem by adding a
> __reduce_ex__() method to a shared ctype object instead of using copy_reg.

I can confirm that the patch fixes the problem on Windows (running test_ctypes
before test_multiprocessing).

However, the patch did not apply cleanly to current SVN trunk - I had to manually
patch the code.  I'll attach the patch that I have now when I run 'svn diff' as
multiprocessing.patch (hope it works, sending as email attachment).
msg68371 - (view) Author: Jesse Noller (jnoller) * (Python committer) Date: 2008-06-18 18:53
Thomas' patch applied and runs clean for me - does anyone have a problem 
with me submitting it?
msg68376 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-06-18 22:03
roudkerk wrote:
> Yes, on Windows pickling is needed to pass data to a child process.  In
> other contexts these objects are NOT picklable because you would have to
> worry about garbage collection of the original object before the copy is
> rebuilt by the other process.  On unix pickling will always fail even if
> it "if win32" statement was removed.

I am not sure to understand. Can you elaborate?
How is memory management different between windows and unix?
msg68415 - (view) Author: roudkerk (roudkerk) Date: 2008-06-19 18:00
> I am not sure to understand. Can you elaborate?
> How is memory management different between windows and unix?

Removing the "if win32" bits will not make shared ctypes objects
picklable on unix.  Even on windows there are only picklable in the
context of spawning a child process.

I do not want to encourage people to try to transfer objects which
wrap operating system resources between running processes using
pickling because it is error prone unless done very carefully: one
needs to find some way of "keeping the resource alive" until the
target process gets a chance to unpickle the data.  (The source
process must not close its handle to the resource until the target
process obtains its own handle, which may not happen for a long time.)

The simplest way to avoid such problems is to only share such
resources through inheritance.  I do add some pickling support to some
types on Windows, but only to emulate the behaviour that Unix gets for
free using fork().  (On Windows trying to transfer objects like locks
or shared ctypes objects over a pipe or queue will, by design, fail
with a RuntimeError)
msg68417 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-06-19 19:19
Why not use a custom Pickler in this case?
It would to be enough to copy the self.dispatch dictionary, and add the
special types there.
msg68431 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-06-20 00:04
Here is a patch that implements a custom pickler for
multiprocessing.forking.
copy_reg has been completely replaced by calls to
ForkingPickler.register(): the global registry is not modified.
msg68808 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-06-26 23:17
Jesse: ping?
msg68810 - (view) Author: Jesse Noller (jnoller) * (Python committer) Date: 2008-06-26 23:40
Sorry - I've been sick and overly busy this week, the mp issues are on  
my asap pile

On Jun 26, 2008, at 7:17 PM, Amaury Forgeot d'Arc <report@bugs.python.org 
 > wrote:

>
> Amaury Forgeot d'Arc <amauryfa@gmail.com> added the comment:
>
> Jesse: ping?
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue3125>
> _______________________________________
msg69104 - (view) Author: Jesse Noller (jnoller) * (Python committer) Date: 2008-07-02 16:46
Tests are back on as of r64663 on trunk.
msg69105 - (view) Author: Jesse Noller (jnoller) * (Python committer) Date: 2008-07-02 16:48
I closed the wrong bug
msg69790 - (view) Author: Jesse Noller (jnoller) * (Python committer) Date: 2008-07-16 14:35
Amaury's patch is applied in r65016 to trunk, all tests pass 

This needs to be merged forward, and then Lib/multiprocessing/managers.py in py3k has to have:

      for view_type in view_types:
         copy_reg.pickle(view_type, rebuild_as_list)

changed to:

      for view_type in view_types:
         ForkingPickler.register(view_type, rebuild_as_list)
msg69818 - (view) Author: Jesse Noller (jnoller) * (Python committer) Date: 2008-07-16 19:21
Here is a proposed patch for py3k generated from an svn merge of amaury's 
patch into py3k. This is currently blocked due to issue3385.
msg72272 - (view) Author: Jesse Noller (jnoller) * (Python committer) Date: 2008-09-01 16:34
Working on the py3k patch now, bumping to rel. blocker
msg72280 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-09-01 17:13
Jesse,
It seems that the patch was merged into py3k by r65883.
The trick was
    from pickle import _Pickler as Pickler
to get the subclassable python implementation.

The only remaining point is the handling of dictionary views
(see rebuild_as_list() in managers.py).
I had to register them with copyreg.pickle, because the C function
connection_send_obj() uses the original pickler.
msg72394 - (view) Author: Jesse Noller (jnoller) * (Python committer) Date: 2008-09-03 16:47
Just to confirm this issue was resolved enough to be lowered from a 
blocker - I ran py3k tests in a loop for about 2+ hours. The fix applied 
in r65883 works well enough that the current implementation does not need 
to change.
msg115091 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2010-08-27 15:19
Is this still a problem or can this be closed, given r65883?
msg115939 - (view) Author: Ask Solem (asksol) (Python committer) Date: 2010-09-09 11:32
As no one has been able to confirm that this is still an issue, I'm closing it as "out of date". The issue can be reopened if necessary.
History
Date User Action Args
2010-09-09 11:32:13asksolsetstatus: open -> closed
resolution: accepted -> out of date
messages: + msg115939
2010-08-27 15:19:26BreamoreBoysetnosy: + BreamoreBoy
messages: + msg115091

components: + Tests
type: behavior
stage: patch review
2010-08-27 12:58:22asksolsetnosy: + asksol
2008-09-03 16:47:56jnollersetpriority: release blocker -> high
messages: + msg72394
versions: + Python 3.1, Python 2.7, - Python 2.6
2008-09-01 17:13:19amaury.forgeotdarcsetmessages: + msg72280
2008-09-01 16:34:08jnollersetpriority: release blocker
messages: + msg72272
2008-07-16 19:21:18jnollersetfiles: + py3k_no_copyreg.patch
messages: + msg69818
2008-07-16 14:35:43jnollersetmessages: + msg69790
2008-07-02 16:48:34jnollersetstatus: closed -> open
resolution: fixed -> accepted
messages: + msg69105
2008-07-02 16:46:58jnollersetstatus: open -> closed
resolution: fixed
messages: + msg69104
2008-06-26 23:40:20jnollersetmessages: + msg68810
2008-06-26 23:17:44amaury.forgeotdarcsetmessages: + msg68808
2008-06-20 00:04:39amaury.forgeotdarcsetfiles: + no_copyreg.patch
messages: + msg68431
2008-06-19 19:19:07amaury.forgeotdarcsetmessages: + msg68417
2008-06-19 18:01:00roudkerksetmessages: + msg68415
2008-06-19 13:34:56jnollersetassignee: jnoller
2008-06-18 22:03:05amaury.forgeotdarcsetmessages: + msg68376
2008-06-18 18:53:19jnollersetmessages: + msg68371
2008-06-18 18:13:45thellersetfiles: + multiprocessing.patch
messages: + msg68369
2008-06-18 17:37:34roudkerksetmessages: + msg68368
2008-06-18 16:10:24amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg68367
2008-06-18 15:45:23roudkerksetfiles: + sharedctypes.py.patch
keywords: + patch
messages: + msg68365
2008-06-16 18:52:10thellersetmessages: + msg68287
2008-06-16 18:47:18jnollersetnosy: + roudkerk
2008-06-16 18:45:52Rhamphoryncussetmessages: + msg68285
2008-06-16 18:37:26thellersetmessages: + msg68284
2008-06-16 18:28:38jnollersetmessages: + msg68283
2008-06-16 18:26:27jnollersetmessages: + msg68282
2008-06-16 18:12:59Rhamphoryncussetmessages: + msg68280
2008-06-16 18:01:48thellersetmessages: + msg68279
2008-06-16 17:53:03jnollersetmessages: + msg68278
2008-06-16 17:47:31Rhamphoryncussetnosy: + Rhamphoryncus, jnoller
2008-06-16 17:31:36thellercreate