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

Multiprocessing.Queue._feed deadlocks on import #67042

Closed
ffinkernagel mannequin opened this issue Nov 12, 2014 · 12 comments
Closed

Multiprocessing.Queue._feed deadlocks on import #67042

ffinkernagel mannequin opened this issue Nov 12, 2014 · 12 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@ffinkernagel
Copy link
Mannequin

ffinkernagel mannequin commented Nov 12, 2014

BPO 22853
Nosy @serhiy-storchaka, @applio
Files
  • show_queue_import_bug.py
  • issue_22853_fix_and_test_import_lock_in_queue_py27.patch: Florian's fix and a matching test for 2.7
  • issue_22853_only_test_import_lock_in_queue_py34_and_py35.patch: Ignore -- replaced by newer patch file.
  • issue_22853_revised_only_test_import_lock_in_queue_py34_and_py35.patch: Regression test-only for 3.4 and 3.5 (both), replaces prior patch for 3.[4-5].
  • 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 = 'https://github.com/serhiy-storchaka'
    closed_at = <Date 2015-03-06.21:36:21.616>
    created_at = <Date 2014-11-12.12:29:31.069>
    labels = ['type-bug', 'library']
    title = 'Multiprocessing.Queue._feed deadlocks on import'
    updated_at = <Date 2015-03-06.21:36:21.615>
    user = 'https://bugs.python.org/ffinkernagel'

    bugs.python.org fields:

    activity = <Date 2015-03-06.21:36:21.615>
    actor = 'serhiy.storchaka'
    assignee = 'serhiy.storchaka'
    closed = True
    closed_date = <Date 2015-03-06.21:36:21.616>
    closer = 'serhiy.storchaka'
    components = ['Library (Lib)']
    creation = <Date 2014-11-12.12:29:31.069>
    creator = 'ffinkernagel'
    dependencies = []
    files = ['37185', '38272', '38273', '38361']
    hgrepos = []
    issue_num = 22853
    keywords = ['patch']
    message_count = 12.0
    messages = ['231073', '236790', '236854', '236865', '237375', '237376', '237377', '237378', '237379', '237382', '237384', '237385']
    nosy_count = 6.0
    nosy_names = ['BreamoreBoy', 'python-dev', 'sbt', 'serhiy.storchaka', 'ffinkernagel', 'davin']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'commit review'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue22853'
    versions = ['Python 2.7']

    @ffinkernagel
    Copy link
    Mannequin Author

    ffinkernagel mannequin commented Nov 12, 2014

    If you import a module that creates a multiprocessing.Queue, puts a value, and then waits for to be received again from the queue, you run into a deadlock.

    The issue is that Queue._feed does 'from .util import is_existing' - which needs the import lock, but is still being held by the main thread.

    Attached a script that illustrates this.

    Patch is a two line change, import is_exiting in line 49, remove the import inside the thread:

    49c49
    < from multiprocessing.util import debug, info, Finalize, register_after_fork
    ---

    from multiprocessing.util import debug, info, Finalize, register_after_fork, is_exiting
    232d231
    < from .util import is_exiting

    @BreamoreBoy
    Copy link
    Mannequin

    BreamoreBoy mannequin commented Feb 27, 2015

    @davin I believe that you're interested in multiprocessing issues.

    @applio
    Copy link
    Member

    applio commented Feb 27, 2015

    Confirmed that the issue can be reproduced under 2.7.9 on OS X 10.10.

    It is not possible to reproduce the issue with default (3.5) -- taking a look at what's different there, notably the import of is_exiting has moved to the top of the queues module and is no longer only imported at the time Queue._feed is invoked, which is just as Florian advocates doing for 2.7.

    I should take a closer look to understand what else has changed.

    @mark: Cool -- thanks.

    @applio applio added the type-bug An unexpected behavior, bug, or error label Feb 27, 2015
    @applio
    Copy link
    Member

    applio commented Feb 28, 2015

    Attaching a patch for 2.7 that applies Florian's fix and provides a test for it as well.

    Although the issue is not triggered on 3.4 or default (3.5), there is the potential for regression there -- attaching a single patch that works for both 3.4 and 3.5 to provide a regression test (only a test, nothing to "fix").

    These patches have been tested on OS X 10.10 and Ubuntu 12.04.5 64-bit for each of 2.7, 3.4, and default (3.5).

    @serhiy-storchaka serhiy-storchaka self-assigned this Mar 6, 2015
    @serhiy-storchaka
    Copy link
    Member

    I'm not sure there is a need to fix this issue. Using multiprocessing Queue at import time looks as yet one way to shoot yourself in the foot. But the patch looks harmful, I'll commit it.

    @serhiy-storchaka serhiy-storchaka added the stdlib Python modules in the Lib dir label Mar 6, 2015
    @BreamoreBoy
    Copy link
    Mannequin

    BreamoreBoy mannequin commented Mar 6, 2015

    That's all we need, harmful patches being committed :) Harmless possibly?

    @serhiy-storchaka
    Copy link
    Member

    Oh, right. Harmless. Thanks for all the fish Mark. I meant the bug.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 6, 2015

    New changeset 069c13ca7a70 by Serhiy Storchaka in branch '2.7':
    Issue bpo-22853: Fixed a deadlock when use multiprocessing.Queue at import time.
    https://hg.python.org/cpython/rev/069c13ca7a70

    @serhiy-storchaka
    Copy link
    Member

    As for 3.x, underscored test does not run, and when remove the underscore it runs, but produce a warning (you should run regrtests with -vv to see detailed warnings):

    $ ./python -m test.regrtest -vv -m '*no_import_lock_contention*' test_multiprocessing_spawn
    ...
    Warning -- threading._dangling was modified by test_multiprocessing_spawn
      Before: <_weakrefset.WeakSet object at 0xb6a960ac>
      After:  <_weakrefset.WeakSet object at 0xb6c4cc0c> 
    1 test altered the execution environment:
        test_multiprocessing_spawn

    @applio
    Copy link
    Member

    applio commented Mar 6, 2015

    Corrected patch for 3.4 and default/3.5 -- newly introduced test is now turned on this time and the dangling weak references are properly addressed as well as the reference to Empty. Nastiness.

    Good save, Serhiy.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 6, 2015

    New changeset cf12856bde17 by Serhiy Storchaka in branch '3.4':
    Issue bpo-22853: Added regression test for using multiprocessing.Queue at import
    https://hg.python.org/cpython/rev/cf12856bde17

    New changeset dcd6d41f2c9a by Serhiy Storchaka in branch 'default':
    Issue bpo-22853: Added regression test for using multiprocessing.Queue at import
    https://hg.python.org/cpython/rev/dcd6d41f2c9a

    @serhiy-storchaka
    Copy link
    Member

    Thank you Florian and Davin for your contribution.

    @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
    stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants