Merge pull request #2814 from rmartin16/task-factory

Hold strong references to asyncio Tasks until they complete (redux)

android/src/toga_android/app.py

@@ -284,7 +284,7 @@ def show_about_dialog(self):
 
         # Create and show an info dialog as the about dialog.
         # We don't care about the response.
-        self.interface._create_task(
+        asyncio.create_task(
             self.interface.dialog(
                 InfoDialog(
                     f"About {self.interface.formal_name}",

android/tests_backend/dialogs.py

@@ -24,7 +24,7 @@ async def _close_dialog():
                     # isn't what as expected, so record that in the future.
                     future.set_exception(e)
 
-            asyncio.ensure_future(_close_dialog())
+            asyncio.create_task(_close_dialog(), name="close-dialog")
 
         dialog._impl.show = automated_show
 

changes/2809.bugfix.rst

@@ -1 +1 @@
-Asynchronous tasks created by Toga are now protected from garbage collection while they are running. This could lead to asynchronous tasks terminating unexpectedly with an error under some conditions.
+Asynchronous tasks are now protected from garbage collection while they are running. This could lead to asynchronous tasks terminating unexpectedly with an error under some conditions.

cocoa/src/toga_cocoa/app.py

@@ -49,7 +49,7 @@ def applicationSupportsSecureRestorableState_(self, app) -> bool:
 
     @objc_method
     def applicationOpenUntitledFile_(self, sender) -> bool:
-        self.interface._create_task(self.interface.documents.request_open())
+        asyncio.create_task(self.interface.documents.request_open())
         return True
 
     @objc_method

cocoa/src/toga_cocoa/dialogs.py

@@ -94,7 +94,7 @@ async def _run_app_modal():
             self.native.window.orderOut(None)
 
         # This needs to be queued as a background task
-        toga.App._create_task(_run_app_modal())
+        asyncio.create_task(_run_app_modal())
 
 
 class InfoDialog(NSAlertDialog):

core/src/toga/app.py

@@ -14,7 +14,7 @@
 
 from toga.command import Command, CommandSet
 from toga.documents import Document, DocumentSet
-from toga.handlers import create_task, simple_handler, wrapped_handler
+from toga.handlers import simple_handler, wrapped_handler
 from toga.hardware.camera import Camera
 from toga.hardware.location import Location
 from toga.icons import Icon
@@ -140,15 +140,14 @@ class App:
     _camera: Camera
     _location: Location
     _main_window: Window | str | None
+    _running_tasks: set[asyncio.Task] = set()
 
     #: A constant that can be used as the main window to indicate that an app will
     #: run in the background without a main window.
     BACKGROUND: str = "background app"
 
     _UNDEFINED: str = "<main window not assigned>"
 
-    _create_task = staticmethod(create_task)
-
     def __init__(
         self,
         formal_name: str | None = None,
@@ -481,6 +480,35 @@ def main_loop(self) -> None:
 
         self._impl.main_loop()
 
+    def _install_task_factory_wrapper(self):
+        """Wrap task creation to track pending and running tasks.
+
+        When tasks are created, asyncio only maintains a weak reference to the task.
+        This allows for the possibility that the garbage collector destroys the task
+        in the midst of its execution. To avoid this, a strong reference is stored on
+        the app and a task callback removes the reference after the task completes.
+        Upstream issue tracked at python/cpython#91887.
+        """
+        platform_task_factory = self.loop.get_task_factory()
+
+        def factory(loop, coro, context=None):
+            if platform_task_factory is not None:
+                if sys.version_info < (3, 11):
+                    task = platform_task_factory(loop, coro)
+                else:
+                    task = platform_task_factory(loop, coro, context=context)
+            else:
+                if sys.version_info < (3, 11):
+                    task = asyncio.Task(coro, loop=loop)
+                else:
+                    task = asyncio.Task(coro, loop=loop, context=context)
+
+            self._running_tasks.add(task)
+            task.add_done_callback(self._running_tasks.discard)
+            return task
+
+        self.loop.set_task_factory(factory)
+
     @property
     def main_window(self) -> Window | str | None:
         """The main window for the app.
@@ -513,7 +541,7 @@ def main_window(self, window: MainWindow | str | None) -> None:
         else:
             raise ValueError(f"Don't know how to use {window!r} as a main window.")
 
-    def _open_initial_document(self, filename: Path) -> bool:
+    def _open_initial_document(self, filename: Path | str) -> bool:
         """Internal utility method for opening a document provided at the command line.
 
         This is abstracted so that backends that have their own management of command
@@ -608,6 +636,9 @@ def _create_initial_windows(self):
                 )
 
     def _startup(self) -> None:
+        # Wrap the platform's event loop's task factory for task tracking
+        self._install_task_factory_wrapper()
+
         # Install the standard commands. This is done *before* startup so the user's
         # code has the opportunity to remove/change the default commands.
         self._create_standard_commands()
@@ -885,16 +916,10 @@ def windows(self, windows: WindowSet) -> None:
 
     def add_background_task(self, handler: BackgroundTask) -> None:
         """**DEPRECATED** – Use :any:`asyncio.create_task`, or override/assign
-        :meth:`~toga.App.on_running`.
-
-        Please review the Python docs for :any:`asyncio.create_task` and follow the
-        advice to save a reference to the returned task in your app.
-        """
+        :meth:`~toga.App.on_running`."""
         warnings.warn(
-            "App.add_background_task() is deprecated. Use asyncio.create_task(), "
-            "or set an App.on_running() handler. Notice the important note for "
-            "asyncio.create_task() at https://docs.python.org/3/library/asyncio-task.html#asyncio.create_task "
-            "to save a reference to the returned task.",
+            "App.add_background_task is deprecated. Use asyncio.create_task(), "
+            "or set an App.on_running() handler",
             DeprecationWarning,
             stacklevel=2,
         )

core/src/toga/handlers.py

@@ -11,22 +11,21 @@
     Any,
     Awaitable,
     Callable,
-    Coroutine,
     Generator,
     NoReturn,
     Protocol,
     TypeVar,
     Union,
 )
 
-T = TypeVar("T")
-
 if TYPE_CHECKING:
     if sys.version_info < (3, 10):
         from typing_extensions import TypeAlias
     else:
         from typing import TypeAlias
 
+    T = TypeVar("T")
+
     GeneratorReturnT = TypeVar("GeneratorReturnT")
     HandlerGeneratorReturnT: TypeAlias = Generator[
         Union[float, None], object, GeneratorReturnT
@@ -38,9 +37,6 @@
     HandlerT: TypeAlias = Union[HandlerSyncT, HandlerAsyncT, HandlerGeneratorT]
     WrappedHandlerT: TypeAlias = Callable[..., object]
 
-# Holds references to asyncio.Tasks created by Toga; see create_task().
-running_tasks = set()
-
 
 def overridable(method: T) -> T:
     """Decorate the method as being user-overridable"""
@@ -57,23 +53,6 @@ def overridden(coroutine_or_method: Callable) -> bool:
     return not hasattr(coroutine_or_method, "_overridden")
 
 
-def create_task(coro_or_future: Coroutine | Awaitable[object]) -> asyncio.Task:
-    """Add a task for a handler to the event loop.
-
-    When Tasks are created, a strong reference should be maintained for it while it is
-    running. The event loop only keeps weak references for the task; so, tracking each
-    Task here ensures the garbage collector will not destroy it mid-execution.
-    Upstream issue tracking at python/cpython#91887.
-
-    :param coro_or_future: Coroutine or Future for the Task to run
-    :returns: the created Task
-    """
-    task = asyncio.ensure_future(coro_or_future)
-    running_tasks.add(task)
-    task.add_done_callback(running_tasks.discard)
-    return task
-
-
 class NativeHandler:
     def __init__(self, handler: Callable[..., object]):
         self.native = handler
@@ -187,7 +166,7 @@ def wrapped_handler(
 
         def _handler(*args: object, **kwargs: object) -> object:
             if asyncio.iscoroutinefunction(handler):
-                return create_task(
+                return asyncio.ensure_future(
                     handler_with_cleanup(handler, cleanup, interface, *args, **kwargs)
                 )
             else:
@@ -198,7 +177,7 @@ def _handler(*args: object, **kwargs: object) -> object:
                     traceback.print_exc()
                 else:
                     if inspect.isgenerator(result):
-                        return create_task(
+                        return asyncio.ensure_future(
                             long_running_task(interface, result, cleanup)
                         )
                     else:

core/tests/app/test_app.py

@@ -347,7 +347,7 @@ def test_explicit_app_metadata(monkeypatch, event_loop):
 
 
 @pytest.mark.parametrize("construct", [True, False])
-def test_icon_construction(construct, event_loop):
+def test_icon_construction(app, construct, event_loop):
     """The app icon can be set during construction."""
     if construct:
         icon = toga.Icon("path/to/icon")
@@ -826,7 +826,7 @@ async def background(app, **kwargs):
         canary()
 
     with pytest.warns(
-        DeprecationWarning, match=r"App.add_background_task\(\) is deprecated"
+        DeprecationWarning, match="App.add_background_task is deprecated"
     ):
         app.add_background_task(background)
 

core/tests/conftest.py

@@ -15,6 +15,15 @@ def reset_global_state():
     toga_window._window_count = -1
 
 
+@pytest.fixture(autouse=True)
+def no_dangling_tasks():
+    """Ensure any tasks for the test were removed when the test finished."""
+    yield
+    if toga.App.app:
+        tasks = toga.App.app._running_tasks
+        assert not tasks, f"the app has dangling tasks: {tasks}"
+
+
 @pytest.fixture(autouse=True)
 def clear_sys_modules(monkeypatch):
     try:
@@ -27,7 +36,20 @@ def clear_sys_modules(monkeypatch):
 
 
 class TestApp(toga.App):
-    pass
+    def startup(self):
+        # Ensure that Toga's task factory is tracking all tasks
+        toga_task_factory = self.loop.get_task_factory()
+
+        def task_factory(loop, coro, context=None):
+            if sys.version_info < (3, 11):
+                task = toga_task_factory(loop, coro)
+            else:
+                task = toga_task_factory(loop, coro, context=context)
+            assert task in self._running_tasks, f"missing task reference for {task}"
+            return task
+
+        self.loop.set_task_factory(task_factory)
+        super().startup()
 
 
 @pytest.fixture