Skip to content

concurrent.futures ThreadPoolExecutor keeps unnecessary references to worker functions. #60488

Closed
Closed
concurrent.futures ThreadPoolExecutor keeps unnecessary references to worker functions.#60488
Labels
stdlibStandard Library Python modules in the Lib/ directorytype-featureA feature request or enhancement

Description

@mdickinson
mdickinson
opened on Oct 19, 2012
BPO 16284
Nosy @brianquinlan, @mdickinson, @pitrou, @asvetlov
Files
  • kill_reference.diff
  • kill_reference_2.diff
  • kill_reference_3.diff
  • issue16284_with_comments.diff
    		•

    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 2012-11-03.13:36:45.805>
created_at = <Date 2012-10-19.10:15:26.514>
labels = ['type-feature', 'library']
title = 'concurrent.futures ThreadPoolExecutor keeps unnecessary references to worker functions.'
updated_at = <Date 2012-11-03.13:52:51.374>
user = 'https://github.com/mdickinson'

    bugs.python.org fields:

    activity = <Date 2012-11-03.13:52:51.374>
actor = 'asvetlov'
assignee = 'none'
closed = True
closed_date = <Date 2012-11-03.13:36:45.805>
closer = 'asvetlov'
components = ['Library (Lib)']
creation = <Date 2012-10-19.10:15:26.514>
creator = 'mark.dickinson'
dependencies = []
files = ['27619', '27633', '27820', '27850']
hgrepos = []
issue_num = 16284
keywords = ['patch']
message_count = 9.0
messages = ['173318', '173388', '173391', '173435', '173693', '174412', '174608', '174617', '174618']
nosy_count = 9.0
nosy_names = ['bquinlan', 'mark.dickinson', 'pitrou', 'schmir', 'jnoller', 'asvetlov', 'python-dev', 'sbt', 'lyapun']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue16284'
versions = ['Python 3.4']

    Activity

    1. mdickinson commented on Oct 19, 2012

      @mdickinson
      mdickinson
      on Oct 19, 2012
      MemberAuthor

      The ThreadPoolExecutor unnecessarily keeps references to _WorkItem objects. With the attached patch (which lacks a test), all tests still pass, and the references are removed as soon as they're no longer needed.

    2. mdickinson
      added
      type-featureA feature request or enhancement
      on Oct 19, 2012

    3. mdickinson commented on Oct 20, 2012

      @mdickinson
      mdickinson
      on Oct 20, 2012
      MemberAuthor

      A new patch (with tests), and a fuller explanation:

      At work, we've got Python talking to a customer's existing COM library; we're using Thomas Heller's 'comtypes' library to do that. Unfortunately, comtypes depends quite a lot on __del__-time cleanup, so reference counting matters. (I'm well aware that this isn't the recommended way to deal with resource cleanup in Python, but rewriting the existing infrastructure isn't a realistic option here.)

      Anyway, it turned out that the concurrent.futures executors were indirectly holding onto references to COM objects, causing issues with our application.

      The attached patch adds a few 'del' statements to remove references that are no longer needed. For the ProcessExecutor, some of those 'del' statements had to go into the multiprocessing.Queue implementation.

      The troublesome pattern (in both multiprocessing and futures) takes the form (simplified):

      def my_worker_function(...):
    ...
    while <exit_condition_not_satisfied>:
        obj = blocking_wait_for_next_item()
        do_processing(obj)
    ...

      The issue is that the reference to obj is kept until the completion of the next blocking wait call. I'm suggesting just adding an extra 'del obj' after 'do_processing(obj)'.

    4. mdickinson
      added
      stdlibStandard Library Python modules in the Lib/ directory
      on Oct 20, 2012

    5. pitrou commented on Oct 20, 2012

      @pitrou
      pitrou
      on Oct 20, 2012
      Member

      Sounds fine to me. You might want to make the test CPython-specific.

    6. brianquinlan commented on Oct 21, 2012

      @brianquinlan
      brianquinlan
      on Oct 21, 2012
      Contributor

      The concurrent.futures stuff looks good to me.

      Could you add a comment explaining why the delete is necessary? And, as Antoine said, the test should be CPython only.

    7. sbt commented on Oct 24, 2012

      @sbt
      sbtmannequin
      on Oct 24, 2012
      Mannequin

      LGTM

    8. asvetlov commented on Nov 1, 2012

      @asvetlov
      asvetlov
      on Nov 1, 2012
      Contributor

      Updated patch to execute tests only for CPython.

    9. lyapun commented on Nov 3, 2012

      @lyapun
      lyapunmannequin
      on Nov 3, 2012
      Mannequin

      Added comments to patch

    10. python-dev commented on Nov 3, 2012

      @python-dev
      python-devmannequin
      on Nov 3, 2012
      Mannequin

      New changeset 70cef0a160cf by Andrew Svetlov in branch 'default':
      Issue bpo-16284: Prevent keeping unnecessary references to worker functions in concurrent.futures ThreadPoolExecutor.
      http://hg.python.org/cpython/rev/70cef0a160cf

    11. asvetlov commented on Nov 3, 2012

      @asvetlov
      asvetlov
      on Nov 3, 2012
      Contributor

      Committed. Thanks.

    12. asvetlov
      closed this as completedon Nov 3, 2012
    13. ezio-melotti
      transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Labels

      stdlibStandard Library Python modules in the Lib/ directorytype-featureA feature request or enhancement

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions