Skip to content

Commit 0b3a283

Browse files
committed
asyncio: hold our own strong refs for tasks and futures
see https://docs.python.org/3.13/library/asyncio-task.html#asyncio.create_task : > Important > > Save a reference to the result of this function, to avoid a task > disappearing mid-execution. The event loop only keeps weak references > to tasks. A task that isn’t referenced elsewhere may get garbage > collected at any time, even before it’s done. For reliable > “fire-and-forget” background tasks, gather them in a collection ref python/cpython#91887 ref beeware/toga#2814
1 parent b88d9f9 commit 0b3a283

File tree

3 files changed

+65
-1
lines changed

3 files changed

+65
-1
lines changed

electrum/network.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -755,7 +755,6 @@ async def tor_probe_task(p):
755755

756756
proxy = self.proxy
757757
if proxy and proxy.enabled and proxy.mode == 'socks5':
758-
# FIXME GC issues? do we need to store the Future?
759758
asyncio.run_coroutine_threadsafe(tor_probe_task(proxy), self.asyncio_loop)
760759

761760
@log_exceptions

electrum/util.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1652,6 +1652,7 @@ def run_event_loop():
16521652
_asyncio_event_loop = None
16531653

16541654
loop.set_exception_handler(on_exception)
1655+
_set_custom_task_factory(loop)
16551656
# loop.set_debug(True)
16561657
stopping_fut = loop.create_future()
16571658
loop_thread = threading.Thread(
@@ -1670,6 +1671,42 @@ def run_event_loop():
16701671
return loop, stopping_fut, loop_thread
16711672

16721673

1674+
_running_asyncio_tasks = set() # type: Set[asyncio.Future]
1675+
def _set_custom_task_factory(loop: asyncio.AbstractEventLoop):
1676+
"""Wrap task creation to track pending and running tasks.
1677+
When tasks are created, asyncio only maintains a weak reference to them.
1678+
Hence, the garbage collector might destroy the task mid-execution.
1679+
To avoid this, we store a strong reference for the task until it completes.
1680+
1681+
Without this, a lot of APIs are basically Heisenbug-generators... e.g.:
1682+
- "asyncio.create_task"
1683+
- "loop.create_task"
1684+
- "asyncio.ensure_future"
1685+
- what about "asyncio.run_coroutine_threadsafe"? not sure if that is safe.
1686+
1687+
related:
1688+
- https://bugs.python.org/issue44665
1689+
- https://github.com/python/cpython/issues/88831
1690+
- https://github.com/python/cpython/issues/91887
1691+
- https://textual.textualize.io/blog/2023/02/11/the-heisenbug-lurking-in-your-async-code/
1692+
- https://github.com/python/cpython/issues/91887#issuecomment-1434816045
1693+
- "Task was destroyed but it is pending!"
1694+
"""
1695+
1696+
platform_task_factory = loop.get_task_factory()
1697+
1698+
def factory(loop_, coro, **kwargs):
1699+
if platform_task_factory is not None:
1700+
task = platform_task_factory(loop_, coro, **kwargs)
1701+
else:
1702+
task = asyncio.Task(coro, loop=loop_, **kwargs)
1703+
_running_asyncio_tasks.add(task)
1704+
task.add_done_callback(_running_asyncio_tasks.discard)
1705+
return task
1706+
1707+
loop.set_task_factory(factory)
1708+
1709+
16731710
class OrderedDictWithIndex(OrderedDict):
16741711
"""An OrderedDict that keeps track of the positions of keys.
16751712

tests/test_util.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import asyncio
12
from datetime import datetime
23
from decimal import Decimal
34

@@ -472,3 +473,30 @@ def test_shortchannelid(self):
472473
self.assertTrue(ShortID.from_components(3, 30, 300) > ShortID.from_components(3, 1, 999))
473474
self.assertTrue(ShortID.from_components(3, 30, 300) < ShortID.from_components(3, 999, 1))
474475

476+
async def test_custom_task_factory(self):
477+
loop = util.get_running_loop()
478+
# set our factory. note: this does not leak into other unit tests
479+
util._set_custom_task_factory(loop)
480+
481+
evt = asyncio.Event()
482+
async def foo():
483+
await evt.wait()
484+
485+
fut = asyncio.ensure_future(foo())
486+
self.assertTrue(fut in util._running_asyncio_tasks)
487+
fut = asyncio.create_task(foo())
488+
self.assertTrue(fut in util._running_asyncio_tasks)
489+
fut = loop.create_task(foo())
490+
self.assertTrue(fut in util._running_asyncio_tasks)
491+
#fut = asyncio.run_coroutine_threadsafe(foobar(), loop=loop)
492+
#self.assertTrue(fut in util._running_asyncio_tasks)
493+
494+
# we should have stored one ref for each above.
495+
# (though what if test framework is doing stuff ~concurrently?)
496+
self.assertEqual(3, len(util._running_asyncio_tasks))
497+
evt.set()
498+
for _ in range(10): # wait a few event loop iterations
499+
await asyncio.sleep(0)
500+
# refs should be cleaned up by now:
501+
self.assertEqual(0, len(util._running_asyncio_tasks))
502+

0 commit comments

Comments
 (0)