Repository navigation
IsolatedAsyncioTestCase __del__ could be dangerous #96021
Description
Activity
Apart from a reproducer it would be nice if your issue description was more clearly stating the problem. Quoting from the middle of another discussion is just confusing.
I had a go at undrafting the issue, I still need to make a reproducer
Hm... I had sort of assumed that debug() was intentionally not cleaning anything up on failure -- TestCase.debug() doesn't either (it has a special case for
@skipthough).PS. There is no such thing as a draft issue.
- added a commit that references this issue
on Aug 24, 2022 - added a commit that references this issue
on Aug 24, 2022 1 remaining item
- added a commit that references this issue
on Aug 25, 2022 What's the status of this? @graingert Do you have a reproducer?
Nothing changed. #96135 solved some potential problems in our tests of
unittestby using a private method, but the original problem still exists.It is not even clear what solution should be.
Reproducer on 3.14.5 (same on main): a failing test's event loop is torn down under
run()but leaked underdebug().import unittest, asyncio, gc loops = [] class Test(unittest.IsolatedAsyncioTestCase): async def test_boom(self): loops.append(asyncio.get_running_loop()) raise ValueError("boom") Test("test_boom").run(unittest.TestResult()) print(loops[-1].is_closed()) # True: run() wraps super().run() in try/finally loops.clear() t = Test("test_boom") try: t.debug() except ValueError: pass print(loops[-1].is_closed()) # False: debug() has no try/finally, runner leaks del t; gc.collect() print(loops[-1].is_closed()) # True: closed only now, by __del__ from the GC
Two separate problems are tangled here:
debug()not tearing down on failure.run()already does; onlydebug()lacks thetry/finally, so making them consistent is a one-line change. TheTestCase.debug()parallel does not really hold: the sync case has no event loop to leak.- The
__del__fallback closing the runner from the GC, on whatever thread collects the case, is theset_event_loop-from-the-wrong-thread hazard from the original report, and it stays regardless of (1).
So (1) is a small consistency fix; (2) is the actual open design question (whether
__del__should run asyncio teardown at all, or only warn).
It looks like in IsolatedAsyncioTestCase when
debug()crashes it doesn't teardown the event loop, in addition it seems that if the loop isn't torn down it will happen in__del__which means thread local sensitive functions likeset_event_loopget called via the GC from potentially the wrong threadOriginally posted by @graingert in #95898 (comment)