Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test_multiprocessing causes test_ctypes to fail #47375

Closed
theller opened this issue Jun 16, 2008 · 29 comments
Closed

test_multiprocessing causes test_ctypes to fail #47375

theller opened this issue Jun 16, 2008 · 29 comments
Labels
tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error

Comments

@theller
Copy link

theller commented Jun 16, 2008

BPO 3125
Nosy @theller, @amauryfa
Files
  • sharedctypes.py.patch
  • multiprocessing.patch
  • no_copyreg.patch
  • py3k_no_copyreg.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2010-09-09.11:32:13.300>
    created_at = <Date 2008-06-16.17:31:36.939>
    labels = ['type-bug', 'tests']
    title = 'test_multiprocessing causes test_ctypes to fail'
    updated_at = <Date 2010-09-09.11:32:13.298>
    user = 'https://github.com/theller'

    bugs.python.org fields:

    activity = <Date 2010-09-09.11:32:13.298>
    actor = 'asksol'
    assignee = 'jnoller'
    closed = True
    closed_date = <Date 2010-09-09.11:32:13.300>
    closer = 'asksol'
    components = ['Tests']
    creation = <Date 2008-06-16.17:31:36.939>
    creator = 'theller'
    dependencies = []
    files = ['10652', '10654', '10661', '10911']
    hgrepos = []
    issue_num = 3125
    keywords = ['patch']
    message_count = 29.0
    messages = ['68276', '68278', '68279', '68280', '68282', '68283', '68284', '68285', '68287', '68365', '68367', '68368', '68369', '68371', '68376', '68415', '68417', '68431', '68808', '68810', '69104', '69105', '69790', '69818', '72272', '72280', '72394', '115091', '115939']
    nosy_count = 7.0
    nosy_names = ['theller', 'amaury.forgeotdarc', 'Rhamphoryncus', 'roudkerk', 'jnoller', 'asksol', 'BreamoreBoy']
    pr_nums = []
    priority = 'high'
    resolution = 'out of date'
    stage = 'patch review'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue3125'
    versions = ['Python 3.1', 'Python 2.7']

    @theller
    Copy link
    Author

    theller commented Jun 16, 2008

    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

    @jnoller
    Copy link
    Mannequin

    jnoller mannequin commented Jun 16, 2008

    Adam, is this due to the c-code tainting you pointed out earlier?

    @theller
    Copy link
    Author

    theller commented Jun 16, 2008

    IMO this problem occurs because multiprocessing registers pickling
    support for ctypes, but the ctypes in trunk already has support for
    pickling.

    @Rhamphoryncus
    Copy link
    Mannequin

    Rhamphoryncus mannequin commented Jun 16, 2008

    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.

    @jnoller
    Copy link
    Mannequin

    jnoller mannequin commented Jun 16, 2008

    Sorry Adam, I was referencing:

    http://bugs.python.org/issue3102

    Also, it is possible issue: http://bugs.python.org/issue3102 is related?

    @jnoller
    Copy link
    Mannequin

    jnoller mannequin commented Jun 16, 2008

    Sorry, the first link should have been:
    http://bugs.python.org/issue3093

    @theller
    Copy link
    Author

    theller commented Jun 16, 2008

    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

    @Rhamphoryncus
    Copy link
    Mannequin

    Rhamphoryncus mannequin commented Jun 16, 2008

    I see no common symbols between bpo-3102 and bpo-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)

    @theller
    Copy link
    Author

    theller commented Jun 16, 2008

    Adam Olsen <rhamph@gmail.com> added the comment:

    I see no common symbols between bpo-3102 and bpo-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>

    @roudkerk
    Copy link
    Mannequin

    roudkerk mannequin commented Jun 18, 2008

    This patch to sharedctypes should fix the problem by adding a
    __reduce_ex__() method to a shared ctype object instead of using copy_reg.

    @amauryfa
    Copy link
    Member

    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.

    @roudkerk
    Copy link
    Mannequin

    roudkerk mannequin commented Jun 18, 2008

    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.

    @theller
    Copy link
    Author

    theller commented Jun 18, 2008

    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).

    @jnoller
    Copy link
    Mannequin

    jnoller mannequin commented Jun 18, 2008

    Thomas' patch applied and runs clean for me - does anyone have a problem
    with me submitting it?

    @amauryfa
    Copy link
    Member

    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?

    @jnoller jnoller mannequin self-assigned this Jun 19, 2008
    @roudkerk
    Copy link
    Mannequin

    roudkerk mannequin commented Jun 19, 2008

    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)

    @amauryfa
    Copy link
    Member

    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.

    @amauryfa
    Copy link
    Member

    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.

    @amauryfa
    Copy link
    Member

    Jesse: ping?

    @jnoller
    Copy link
    Mannequin

    jnoller mannequin commented Jun 26, 2008

    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\>


    @jnoller
    Copy link
    Mannequin

    jnoller mannequin commented Jul 2, 2008

    Tests are back on as of r64663 on trunk.

    @jnoller jnoller mannequin closed this as completed Jul 2, 2008
    @jnoller
    Copy link
    Mannequin

    jnoller mannequin commented Jul 2, 2008

    I closed the wrong bug

    @jnoller jnoller mannequin reopened this Jul 2, 2008
    @jnoller
    Copy link
    Mannequin

    jnoller mannequin commented Jul 16, 2008

    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)

    @jnoller
    Copy link
    Mannequin

    jnoller mannequin commented Jul 16, 2008

    Here is a proposed patch for py3k generated from an svn merge of amaury's
    patch into py3k. This is currently blocked due to bpo-3385.

    @jnoller
    Copy link
    Mannequin

    jnoller mannequin commented Sep 1, 2008

    Working on the py3k patch now, bumping to rel. blocker

    @jnoller jnoller mannequin added the release-blocker label Sep 1, 2008
    @amauryfa
    Copy link
    Member

    amauryfa commented Sep 1, 2008

    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.

    @jnoller
    Copy link
    Mannequin

    jnoller mannequin commented Sep 3, 2008

    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.

    @jnoller jnoller mannequin removed the release-blocker label Sep 3, 2008
    @BreamoreBoy
    Copy link
    Mannequin

    BreamoreBoy mannequin commented Aug 27, 2010

    Is this still a problem or can this be closed, given r65883?

    @BreamoreBoy BreamoreBoy mannequin added tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error labels Aug 27, 2010
    @asksol
    Copy link
    Mannequin

    asksol mannequin commented Sep 9, 2010

    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.

    @asksol asksol mannequin closed this as completed Sep 9, 2010
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants