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

asyncio.Future.set_exception() creates a reference cycle #64231

Closed
vstinner opened this issue Dec 20, 2013 · 10 comments
Closed

asyncio.Future.set_exception() creates a reference cycle #64231

vstinner opened this issue Dec 20, 2013 · 10 comments

Comments

@vstinner
Copy link
Member

BPO 20032
Nosy @gvanrossum, @pitrou, @vstinner
Files
  • asyncio_break_ref_cycle.patch
  • never_deleted.py
  • 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 2013-12-20.22:37:23.167>
    created_at = <Date 2013-12-20.09:55:03.882>
    labels = ['invalid']
    title = 'asyncio.Future.set_exception() creates a reference cycle'
    updated_at = <Date 2015-12-09.09:35:32.462>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2015-12-09.09:35:32.462>
    actor = 'Jean-Louis Fuchs'
    assignee = 'none'
    closed = True
    closed_date = <Date 2013-12-20.22:37:23.167>
    closer = 'vstinner'
    components = []
    creation = <Date 2013-12-20.09:55:03.882>
    creator = 'vstinner'
    dependencies = []
    files = ['33228', '33238']
    hgrepos = []
    issue_num = 20032
    keywords = ['patch']
    message_count = 10.0
    messages = ['206672', '206674', '206694', '206703', '206705', '206709', '206711', '206712', '206713', '256148']
    nosy_count = 4.0
    nosy_names = ['gvanrossum', 'pitrou', 'vstinner', 'Jean-Louis Fuchs']
    pr_nums = []
    priority = 'normal'
    resolution = 'not a bug'
    stage = None
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue20032'
    versions = ['Python 3.4']

    @vstinner
    Copy link
    Member Author

    asyncio.Future.set_exception(exc) sets the exception attribute to exc, but exc.__traceback__ refers to frames and the current frame probably referes to the future instance.

    Tell me if I'm wrong, but it looks like a reference cycle:
    fut -- fut.exception --> exception --exception.__traceback__ -> traceback --traceback.tb_frame --> frame --frame.fb_locals --> fut

    The frame class got a new clear() method in Python 3.4:
    http://docs.python.org/dev/reference/datamodel.html#frame.clear

    Maybe because of the PEP-442, the reference cycle is no more an issue. In fact, the following example calls fut destructor immediatly, at "fut = None" line.
    ---

    import asyncio
    
    fut = asyncio.Future()
    try:
        raise ValueError()
    except Exception as err:
        fut.set_exception(err)
    fut = None

    Attached patch breaks explicitly the reference cycle by scheduling a call to traceback.clear_frames() using call_soon(). The patch depends on asyncio_defer_format_tb.patch which is attached to the issue bpo-19967.

    @vstinner
    Copy link
    Member Author

    asyncio_break_ref_cycle.patch does not fix the issue on Python 3.3 (for Tulip).

    @gvanrossum
    Copy link
    Member

    Do you have an example of code that behaves differently with this patch? I can't find any.

    @pitrou
    Copy link
    Member

    pitrou commented Dec 20, 2013

    + self._loop.call_soon(traceback.clear_frames,
    + self._exception.__traceback__)

    This will keep the traceback alive until called by the event loop, even if self._exception is cleared in the meantime...

    @vstinner
    Copy link
    Member Author

    Do you have an example of code that behaves differently with this patch? I can't find any.

    I didn't check in the Python standard library, but the reference cycle is obvious, and I hate such issue. It introduces tricky issues like memory leaks.

    Here is an example to demonstrate the issue. The "DELETE OBJECT" message is never displayed, so the object is never deleted (memory leak).

    Comment "fut.set_exception(err)" line to delete the object, or apply attached patch.

    @gvanrossum
    Copy link
    Member

    The cycle will be cleaned up (and the message printed) when the
    garbage collector runs next. Your demo doesn't do anything else, so it
    never allocates memory, so it never runs gc.collect(). But that's only
    because it's a toy program.

    Maybe it's time to look into
    http://code.google.com/p/tulip/issues/detail?id=42 ? (It proposes to
    run gc.collect() occasionally when the loop is idle.)

    I am also concerned about Antoine's point -- the patch may actually
    *prolong* the life of the traceback.

    On Fri, Dec 20, 2013 at 2:15 PM, STINNER Victor <report@bugs.python.org> wrote:

    STINNER Victor added the comment:

    > Do you have an example of code that behaves differently with this patch? I can't find any.

    I didn't check in the Python standard library, but the reference cycle is obvious, and I hate such issue. It introduces tricky issues like memory leaks.

    Here is an example to demonstrate the issue. The "DELETE OBJECT" message is never displayed, so the object is never deleted (memory leak).

    Comment "fut.set_exception(err)" line to delete the object, or apply attached patch.

    ----------
    Added file: http://bugs.python.org/file33238/never_deleted.py


    Python tracker <report@bugs.python.org>
    <http://bugs.python.org/issue20032\>


    @pitrou
    Copy link
    Member

    pitrou commented Dec 20, 2013

    Maybe it's time to look into
    http://code.google.com/p/tulip/issues/detail?id=42 ? (It proposes to
    run gc.collect() occasionally when the loop is idle.)

    Is it possible to break the cycle instead? Or is the graph of references
    too complex for that?

    @gvanrossum
    Copy link
    Member

    The only reasonable place to break the cycle seems to be the frame containing the set_exception() call -- but that could be app code.

    Looking again at what the patch actually does I think it is too big a hammer anyway -- it would break debugging tools that preserve tracebacks and inspect the frames later.

    @vstinner
    Copy link
    Member Author

    "The cycle will be cleaned up (and the message printed) when the
    garbage collector runs next."

    Oh, ok. Using the following task, the object is correctly deleted.
    ---

    @asyncio.coroutine
    def idle():
        while 1:
            gc.collect()
            yield from asyncio.sleep(0.1)
    
    asyncio.Task(idle())

    "Maybe it's time to look into
    http://code.google.com/p/tulip/issues/detail?id=42 ? (It proposes to
    run gc.collect() occasionally when the loop is idle.)"

    I don't like such task. The issue can be documented, maybe with an example of call calling gc.collect() regulary? Such background task should be implemented in the application to control when the garbage collector is called.

    @Jean-LouisFuchs
    Copy link
    Mannequin

    Jean-LouisFuchs mannequin commented Dec 9, 2015

    Just to let you know I hit this problem in my code, simplified version:

    https://gist.github.com/ganwell/ce3718e5119c6e7e9b3e

    Of course it is only a problem because I am a ref-counting stickler.

    @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
    None yet
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants