tornadoweb / tornadoweb/tornado

Possible leak when exception is raised in inner coroutine

Open
#3,346 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

gen
Dominant language
Python
Stars
22.2k
Forks
5.6k
Avg merge
3h 42m
Merged PRs (30d)
16

Description

Hi, I have been debugging a memory leak in an application that uses Tornado and I was able to narrow down the behavior to what I believe is a representative minimum example, based on an existing unit test.

Edit:

  • Reproducing with CPython 3.9.16 on macOS, although the original issue was on Linux with CPython 3.6.8 (I know, super old! but hopefully any workaround would still apply).
  • Tested against master as of a6dfd70d7a6398b2187021df63ca77daa5781e5e but from what I can tell this reproduces very far back to old versions of Tornado (e.g. 4.5+)

Possibly related, from what I can gather (although closed for a long time now): #1872 #2229

I have a couple questions:

  1. Is this expected behavior / is there some reason I'm missing for why a reference cycle forms here? I don't have a ton of experience with Tornado directly so maybe this is just a side effect of how the IOLoop is implemented or something, and it's expected that GC is needed to clean up in this scenario.
  2. Is there a "correct" workaround that would help free the exception / traceback, etc.? I can del the local variables, but my fear is that in that case the remaining stack frames, tracebacks etc. would stick around, possibly resulting in other leaked variables that aren't deleted (particularly if the exception is bubbled through several coroutines in a more complicated example).

Here is the test I'm reproducing with, adapted from the existing one in gen_test.py:

    @skipNotCPython
    @unittest.skipIf(
        (3,) < sys.version_info < (3, 6), "asyncio.Future has reference cycles"
    )
    def test_coroutine_refcounting(self):
        # On CPython, tasks and their arguments should be released immediately
        # without waiting for garbage collection.
        @gen.coroutine
        def raise_exc():
            yield
            raise ValueError("Some error")

        @gen.coroutine
        def inner():
            class Foo(object):
                pass

            local_var = Foo()
            self.local_ref = weakref.ref(local_var)

            try:
                yield raise_exc()
            except ValueError:
                # del local_var # <- this works to free the variable
                pass

        @gen.coroutine
        def inner2():
            yield inner()

            # Error! There is a still a strong reference to the local variable via
            # the ValueError traceback -> `inner` frame, but why is that?
            self.assertIsNone(self.local_ref())

        self.io_loop.run_sync(inner2, timeout=3)

        self.assertIsNone(self.local_ref())
        self.finished = True

Thanks!

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the adapted test_coroutine_refcounting test in gen_test.py and reproduce it on the reported CPython versions. Trace the ValueError traceback and coroutine lifetime around inner() to determine why local_var remains strongly referenced; done means the behavior is explained and the test no longer retains the object unexpectedly.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.