classification
Title: asyncio.Future.set_exception() creates a reference cycle
Type: Stage:
Components: Versions: Python 3.4
process
Status: closed Resolution: not a bug
Dependencies: Superseder:
Assigned To: Nosy List: Jean-Louis Fuchs, gvanrossum, pitrou, vstinner
Priority: normal Keywords: patch

Created on 2013-12-20 09:55 by vstinner, last changed 2015-12-09 09:35 by Jean-Louis Fuchs. This issue is now closed.

Files
File name Uploaded Description Edit
asyncio_break_ref_cycle.patch vstinner, 2013-12-20 09:55
never_deleted.py vstinner, 2013-12-20 22:15
Messages (10)
msg206672 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2013-12-20 09:55
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 #19967.
msg206674 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2013-12-20 09:57
asyncio_break_ref_cycle.patch does not fix the issue on Python 3.3 (for Tulip).
msg206694 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2013-12-20 17:01
Do you have an example of code that behaves differently with this patch?  I can't find any.
msg206703 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-12-20 21:47
+            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...
msg206705 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2013-12-20 22:15
> 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.
msg206709 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2013-12-20 22:23
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>
> _______________________________________
msg206711 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-12-20 22:27
> 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?
msg206712 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2013-12-20 22:35
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.
msg206713 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2013-12-20 22:37
"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.
msg256148 - (view) Author: Jean-Louis Fuchs (Jean-Louis Fuchs) Date: 2015-12-09 09:35
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.
History
Date User Action Args
2015-12-09 09:35:32Jean-Louis Fuchssetnosy: + Jean-Louis Fuchs
messages: + msg256148
2013-12-20 22:37:23vstinnersetstatus: open -> closed
resolution: not a bug
2013-12-20 22:37:14vstinnersetmessages: + msg206713
2013-12-20 22:35:49gvanrossumsetmessages: + msg206712
2013-12-20 22:27:53pitrousetmessages: + msg206711
2013-12-20 22:23:30gvanrossumsetmessages: + msg206709
2013-12-20 22:15:01vstinnersetfiles: + never_deleted.py

messages: + msg206705
2013-12-20 21:47:02pitrousetmessages: + msg206703
2013-12-20 17:01:21gvanrossumsetmessages: + msg206694
2013-12-20 09:57:13vstinnersetmessages: + msg206674
2013-12-20 09:55:03vstinnercreate