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

Raising an exception raised in a "future" instance will create reference cycles #80111

Closed
jcea opened this issue Feb 7, 2019 · 9 comments
Closed
Assignees
Labels
3.8 only security fixes 3.9 only security fixes 3.10 only security fixes

Comments

@jcea
Copy link
Member

jcea commented Feb 7, 2019

BPO 35930
Nosy @jcea, @ned-deily, @miss-islington
PRs
  • bpo-35930: Raising an exception raised in a "future" instance will create reference cycles #24995
  • [3.9] bpo-35930: Raising an exception raised in a "future" instance will create reference cycles (GH-24995) #25070
  • [3.8] bpo-35930: Raising an exception raised in a "future" instance will create reference cycles (GH-24995) #25071
  • [3.7] bpo-35930: Raising an exception raised in a "future" instance will create reference cycles (GH-24995) #25072
  • [3.6] bpo-35930: Raising an exception raised in a "future" instance will create reference cycles (GH-24995) #25073
  • 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/jcea'
    closed_at = <Date 2021-03-29.17:55:16.680>
    created_at = <Date 2019-02-07.14:25:30.179>
    labels = ['3.8', '3.9', '3.10']
    title = 'Raising an exception raised in a "future" instance will create reference cycles'
    updated_at = <Date 2021-04-26.12:25:49.419>
    user = 'https://github.com/jcea'

    bugs.python.org fields:

    activity = <Date 2021-04-26.12:25:49.419>
    actor = 'jcea'
    assignee = 'jcea'
    closed = True
    closed_date = <Date 2021-03-29.17:55:16.680>
    closer = 'jcea'
    components = []
    creation = <Date 2019-02-07.14:25:30.179>
    creator = 'jcea'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 35930
    keywords = ['patch']
    message_count = 9.0
    messages = ['335022', '383099', '383112', '389714', '389717', '389718', '389779', '389780', '391907']
    nosy_count = 3.0
    nosy_names = ['jcea', 'ned.deily', 'miss-islington']
    pr_nums = ['24995', '25070', '25071', '25072', '25073']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue35930'
    versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']

    @jcea
    Copy link
    Member Author

    jcea commented Feb 7, 2019

    Try this in a terminal:

    """
    import gc
    import concurrent.futures
    executor = concurrent.futures.ThreadPoolExecutor(999)

    def a():
      1/0
    
    future=executor.submit(a)
    future.result()
    # An exception is raised here. That is normal
    gc.set_debug(gc.DEBUG_SAVEALL)
    gc.collect()
    gc.garbage
    """

    You will see (python 3.7) that 23 objects are collected when cleaning the cycle.

    The problem is the attribute "future._exception". If the exception provided by the "future" is raised somewhere else, we will have reference cycles because we have the same exception/traceback in two different places in the traceback framestack.

    I commonly do this in my code:

    """
    try:
    future.result() # This will raise an exception if the future did it
    except Exception:
    ... some clean up ...
    raise # Propagate the "future" exception
    """

    This approach will create reference cycles. They will eventually cleaned up, but I noticed this issue because the cycle clean up phase was touching big objects with many references but unused for a long time, so they were living in the SWAP. The cycle collection was hugely slow because of this and the interpreter is completely stopped until done.

    Not sure about what to do about this. I am currently doing something like:

    """
    try:
    future.result() # This will raise an exception if the future did it
    except Exception:
    if future.done():
    del future._exception
    raise # Propagate the exception
    """

    I am breaking the cycle manually. I do not use "future.set_exception(None) because side effects like notifying waiters.

    I think this is a bug to be solved. Not sure how to do it cleanly.

    What do you think? Ideas?.

    @jcea jcea added the 3.7 (EOL) end of life label Feb 7, 2019
    @jcea
    Copy link
    Member Author

    jcea commented Dec 15, 2020

    The corrected test case in the terminal: (a "del" was missing)

    """
    import gc
    import concurrent.futures
    executor = concurrent.futures.ThreadPoolExecutor(999)

    def a():
      1/0
    
    future=executor.submit(a)
    future.result()
    # An exception is raised here. That is normal
    del future  # Variable vanish, but data is still there because the cycle
    gc.set_debug(gc.DEBUG_SAVEALL)
    gc.collect()
    gc.garbage
    """

    @jcea
    Copy link
    Member Author

    jcea commented Dec 16, 2020

    Even more reproductible case, now 100%:

    """
    import gc
    import concurrent.futures
    executor = concurrent.futures.ThreadPoolExecutor(999)

    def a():
      1/0
    
    future=executor.submit(a)
    future.result()
    # An exception is raised here. That is normal
    del future  # Variable vanish, but data is still there because the cycle
    1/0  # Raises another exception drop references to the future one
    gc.set_debug(gc.DEBUG_SAVEALL)
    gc.collect()
    gc.garbage
    """

    @jcea jcea added 3.8 only security fixes 3.9 only security fixes labels Dec 16, 2020
    @jcea jcea self-assigned this Mar 23, 2021
    @jcea jcea added the 3.10 only security fixes label Mar 23, 2021
    @jcea
    Copy link
    Member Author

    jcea commented Mar 29, 2021

    New changeset 32430aa by Jesús Cea in branch 'master':
    bpo-35930: Raising an exception raised in a "future" instance will create reference cycles (bpo-24995)
    32430aa

    @jcea
    Copy link
    Member Author

    jcea commented Mar 29, 2021

    New changeset dae1963 by Miss Islington (bot) in branch '3.8':
    bpo-35930: Raising an exception raised in a "future" instance will create reference cycles (GH-24995) (bpo-25071)
    dae1963

    @jcea
    Copy link
    Member Author

    jcea commented Mar 29, 2021

    New changeset d914813 by Miss Islington (bot) in branch '3.9':
    bpo-35930: Raising an exception raised in a "future" instance will create reference cycles (GH-24995) (bpo-25070)
    d914813

    @jcea jcea closed this as completed Mar 29, 2021
    @ned-deily
    Copy link
    Member

    @jcea, I see you have created backports for 3.7 and 3.6 as well. Those release as in their security-fix-only phase of their life cycles. This doesn't seem like a security issue but am I missing something?

    @ned-deily
    Copy link
    Member

    "Those releases are in their"

    @jcea
    Copy link
    Member Author

    jcea commented Apr 26, 2021

    I retired the backporting request. Thanks, Ned.

    @jcea jcea removed the 3.7 (EOL) end of life label Apr 26, 2021
    @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
    3.8 only security fixes 3.9 only security fixes 3.10 only security fixes
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants