From 4c25750ff19ba1561ee65cb17fecc527d40ab9f3 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Thu, 30 May 2024 16:17:01 +0100 Subject: [PATCH 1/8] timers --- src/textual/app.py | 7 ++++--- src/textual/message_pump.py | 5 +++-- src/textual/reactive.py | 7 +++++++ src/textual/timer.py | 38 +++++++++++++++++++++++++++++-------- src/textual/widget.py | 4 +++- 5 files changed, 47 insertions(+), 14 deletions(-) diff --git a/src/textual/app.py b/src/textual/app.py index a0b914a9ea..a19d71b588 100644 --- a/src/textual/app.py +++ b/src/textual/app.py @@ -111,6 +111,7 @@ _SystemModalScreen, ) from .signal import Signal +from .timer import Timer from .widget import AwaitMount, Widget from .widgets._toast import ToastRack from .worker import NoActiveWorker, get_current_worker @@ -2522,8 +2523,7 @@ async def invoke_ready_callback() -> None: try: await self.animator.stop() finally: - for timer in list(self._timers): - timer.stop() + await Timer._stop_all(self._timers) self._running = True try: @@ -3364,7 +3364,8 @@ async def _prune_nodes(self, widgets: list[Widget]) -> None: """ async with self._dom_lock: for widget in widgets: - await self._prune_node(widget) + if self.is_attached: + await self._prune_node(widget) async def _prune_node(self, root: Widget) -> None: """Remove a node and its children. Children are removed before parents. diff --git a/src/textual/message_pump.py b/src/textual/message_pump.py index 8b7fd18dca..c6ef9decd4 100644 --- a/src/textual/message_pump.py +++ b/src/textual/message_pump.py @@ -519,8 +519,7 @@ async def _process_messages(self) -> None: pass finally: self._running = False - for timer in list(self._timers): - timer.stop() + await Timer._stop_all(self._timers) async def _pre_process(self) -> bool: """Procedure to run before processing messages. @@ -849,6 +848,8 @@ def get_key_handler(pump: MessagePump, key: str) -> Callable | None: return handled async def on_timer(self, event: events.Timer) -> None: + if not self.app._running: + return event.prevent_default() event.stop() if event.callback is not None: diff --git a/src/textual/reactive.py b/src/textual/reactive.py index bab60a971a..f49af60316 100644 --- a/src/textual/reactive.py +++ b/src/textual/reactive.py @@ -73,6 +73,13 @@ def invoke_watcher( value: The new value of the attribute. """ _rich_traceback_omit = True + + from ._context import active_app + + app = active_app.get() + if not app._running: + return + param_count = count_parameters(watch_function) reset_token = active_message_pump.set(watcher_object) try: diff --git a/src/textual/timer.py b/src/textual/timer.py index bf086262d0..0dffc7fc9a 100644 --- a/src/textual/timer.py +++ b/src/textual/timer.py @@ -7,8 +7,8 @@ from __future__ import annotations import weakref -from asyncio import CancelledError, Event, Task, create_task -from typing import Any, Awaitable, Callable, Union +from asyncio import CancelledError, Event, Task, create_task, gather +from typing import Any, Awaitable, Callable, Iterable, Union from rich.repr import Result, rich_repr @@ -83,12 +83,34 @@ def _start(self) -> None: """Start the timer.""" self._task = create_task(self._run_timer(), name=self.name) - def stop(self) -> None: - """Stop the timer.""" - if self._task is not None: - self._active.set() - self._task.cancel() - self._task = None + def stop(self) -> Task: + """Stop the timer. + + Returns: + A Task object. Await this to wait until the timer has completed. + + """ + if self._task is None: + raise RuntimeError("Timer has not been started.") + self._active.set() + self._task.cancel() + return self._task + + @classmethod + async def _stop_all(cls, timers: Iterable[Timer]) -> None: + """Stop a number of timers, and await their completion. + + Args: + timers: A number of timers. + """ + + async def stop_timer(timer) -> None: + if timer._task is not None: + timer._active.set() + timer._task.cancel() + await timer._task + + await gather(*[stop_timer(timer) for timer in list(timers)]) def pause(self) -> None: """Pause the timer. diff --git a/src/textual/widget.py b/src/textual/widget.py index 49f6afa534..e7e87ecbf4 100644 --- a/src/textual/widget.py +++ b/src/textual/widget.py @@ -935,6 +935,7 @@ def mount( provided a ``MountError`` will be raised. """ if not self.is_attached: + return AwaitMount(self, []) raise MountError(f"Can't mount widget(s) before {self!r} is mounted") # Check for duplicate IDs in the incoming widgets ids_to_mount = [widget.id for widget in widgets if widget.id is not None] @@ -3716,7 +3717,8 @@ async def _compose(self) -> None: self.app._handle_exception(error) else: self._extend_compose(widgets) - await self.mount_composed_widgets(widgets) + if widgets: + await self.mount_composed_widgets(widgets) async def mount_composed_widgets(self, widgets: list[Widget]) -> None: """Called by Textual to mount widgets after compose. From 52ac87ac29bd797d7d4372997e6df23b0509d039 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Thu, 30 May 2024 16:25:50 +0100 Subject: [PATCH 2/8] restore sanity check --- src/textual/app.py | 5 +++-- src/textual/timer.py | 7 ++++++- src/textual/widget.py | 1 - 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/textual/app.py b/src/textual/app.py index a19d71b588..4cda806968 100644 --- a/src/textual/app.py +++ b/src/textual/app.py @@ -3362,10 +3362,11 @@ async def _prune_nodes(self, widgets: list[Widget]) -> None: Args: widgets: Widgets to remove. """ + if not self.is_attached: + return async with self._dom_lock: for widget in widgets: - if self.is_attached: - await self._prune_node(widget) + await self._prune_node(widget) async def _prune_node(self, root: Widget) -> None: """Remove a node and its children. Children are removed before parents. diff --git a/src/textual/timer.py b/src/textual/timer.py index 0dffc7fc9a..3845e0183c 100644 --- a/src/textual/timer.py +++ b/src/textual/timer.py @@ -104,7 +104,12 @@ async def _stop_all(cls, timers: Iterable[Timer]) -> None: timers: A number of timers. """ - async def stop_timer(timer) -> None: + async def stop_timer(timer: Timer) -> None: + """Stop a timer and wait for it to finish. + + Args: + timer: A Timer instance. + """ if timer._task is not None: timer._active.set() timer._task.cancel() diff --git a/src/textual/widget.py b/src/textual/widget.py index e7e87ecbf4..459a64e5c4 100644 --- a/src/textual/widget.py +++ b/src/textual/widget.py @@ -935,7 +935,6 @@ def mount( provided a ``MountError`` will be raised. """ if not self.is_attached: - return AwaitMount(self, []) raise MountError(f"Can't mount widget(s) before {self!r} is mounted") # Check for duplicate IDs in the incoming widgets ids_to_mount = [widget.id for widget in widgets if widget.id is not None] From a7079cc5caecb6724bc21cdecda68d4d51b9ee69 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Thu, 30 May 2024 16:26:37 +0100 Subject: [PATCH 3/8] changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 90e4cfb1a0..42c5ba76c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). +## Unreleased + +### Fixed + +- Fix traceback on exit https://github.com/Textualize/textual/pull/4575 + ## [0.63.6] - 2024-05-29 ### Fixed From f663bca524f9cd987fbe7c4c103d9529e706cacf Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Thu, 30 May 2024 16:45:57 +0100 Subject: [PATCH 4/8] fix --- src/textual/timer.py | 7 ++++++- src/textual/widget.py | 6 +++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/textual/timer.py b/src/textual/timer.py index 3845e0183c..6cc664369f 100644 --- a/src/textual/timer.py +++ b/src/textual/timer.py @@ -91,7 +91,12 @@ def stop(self) -> Task: """ if self._task is None: - raise RuntimeError("Timer has not been started.") + + async def noop() -> None: + """A dummy task.""" + + return create_task(noop()) + self._active.set() self._task.cancel() return self._task diff --git a/src/textual/widget.py b/src/textual/widget.py index 459a64e5c4..43e31b265c 100644 --- a/src/textual/widget.py +++ b/src/textual/widget.py @@ -3716,8 +3716,7 @@ async def _compose(self) -> None: self.app._handle_exception(error) else: self._extend_compose(widgets) - if widgets: - await self.mount_composed_widgets(widgets) + await self.mount_composed_widgets(widgets) async def mount_composed_widgets(self, widgets: list[Widget]) -> None: """Called by Textual to mount widgets after compose. @@ -3729,7 +3728,8 @@ async def mount_composed_widgets(self, widgets: list[Widget]) -> None: Args: widgets: A list of child widgets. """ - await self.mount_all(widgets) + if widgets: + await self.mount_all(widgets) def _extend_compose(self, widgets: list[Widget]) -> None: """Hook to extend composed widgets. From b203326dc1c5849d696e53d526fe2b0311cd6301 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Thu, 30 May 2024 17:45:50 +0100 Subject: [PATCH 5/8] wip --- src/textual/app.py | 3 +-- src/textual/message_pump.py | 10 ++++++---- src/textual/signal.py | 2 +- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/textual/app.py b/src/textual/app.py index 4cda806968..ccbe85d3cc 100644 --- a/src/textual/app.py +++ b/src/textual/app.py @@ -3362,8 +3362,6 @@ async def _prune_nodes(self, widgets: list[Widget]) -> None: Args: widgets: Widgets to remove. """ - if not self.is_attached: - return async with self._dom_lock: for widget in widgets: await self._prune_node(widget) @@ -3375,6 +3373,7 @@ async def _prune_node(self, root: Widget) -> None: root: Node to remove. """ # Pruning a node that has been removed is a no-op + if root not in self._registry: return diff --git a/src/textual/message_pump.py b/src/textual/message_pump.py index c6ef9decd4..b3fed6af72 100644 --- a/src/textual/message_pump.py +++ b/src/textual/message_pump.py @@ -247,8 +247,9 @@ def app(self) -> "App[object]": @property def is_attached(self) -> bool: """Is this node linked to the app through the DOM?""" + if self.is_dom_root: + return True node: MessagePump | None = self - while (node := node._parent) is not None: if node.is_dom_root: return True @@ -479,9 +480,10 @@ async def _close_messages(self, wait: bool = True) -> None: if self._closed or self._closing: return self._closing = True - stop_timers = list(self._timers) - for timer in stop_timers: - timer.stop() + await Timer._stop_all(self._timers) + # stop_timers = list(self._timers) + # for timer in stop_timers: + # timer.stop() self._timers.clear() await self._message_queue.put(events.Unmount()) Reactive._reset_object(self) diff --git a/src/textual/signal.py b/src/textual/signal.py index 8fe5d43a23..7128b5530b 100644 --- a/src/textual/signal.py +++ b/src/textual/signal.py @@ -109,7 +109,7 @@ def publish(self, data: SignalT) -> None: """ for node, callbacks in list(self._subscriptions.items()): - if not node.is_running: + if not (node.is_running and node.is_attached): # Removed nodes that are no longer running self._subscriptions.pop(node) else: From 4ff8d4069514372192915487e988a63d6ab068f2 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Sun, 2 Jun 2024 15:11:02 +0100 Subject: [PATCH 6/8] fix exception on exit --- src/textual/reactive.py | 6 ------ src/textual/widget.py | 13 +++++++------ 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/src/textual/reactive.py b/src/textual/reactive.py index f49af60316..c9b153a743 100644 --- a/src/textual/reactive.py +++ b/src/textual/reactive.py @@ -74,12 +74,6 @@ def invoke_watcher( """ _rich_traceback_omit = True - from ._context import active_app - - app = active_app.get() - if not app._running: - return - param_count = count_parameters(watch_function) reset_token = active_message_pump.set(watcher_object) try: diff --git a/src/textual/widget.py b/src/textual/widget.py index 43e31b265c..e5de1959e5 100644 --- a/src/textual/widget.py +++ b/src/textual/widget.py @@ -934,15 +934,16 @@ def mount( Only one of ``before`` or ``after`` can be provided. If both are provided a ``MountError`` will be raised. """ + if self._closing: + return AwaitMount(self, []) if not self.is_attached: raise MountError(f"Can't mount widget(s) before {self!r} is mounted") # Check for duplicate IDs in the incoming widgets - ids_to_mount = [widget.id for widget in widgets if widget.id is not None] - unique_ids = set(ids_to_mount) - num_unique_ids = len(unique_ids) - num_widgets_with_ids = len(ids_to_mount) - if num_unique_ids != num_widgets_with_ids: - counter = Counter(widget.id for widget in widgets) + ids_to_mount = [ + widget_id for widget in widgets if (widget_id := widget.id) is not None + ] + if len(set(ids_to_mount)) != len(ids_to_mount): + counter = Counter(ids_to_mount) for widget_id, count in counter.items(): if count > 1: raise MountError( From 0a6d96b8b12e37b337aedab15a33cc7ee00babfa Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Sun, 2 Jun 2024 15:13:42 +0100 Subject: [PATCH 7/8] remove comments --- src/textual/message_pump.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/textual/message_pump.py b/src/textual/message_pump.py index b3fed6af72..ee4482bad1 100644 --- a/src/textual/message_pump.py +++ b/src/textual/message_pump.py @@ -481,9 +481,6 @@ async def _close_messages(self, wait: bool = True) -> None: return self._closing = True await Timer._stop_all(self._timers) - # stop_timers = list(self._timers) - # for timer in stop_timers: - # timer.stop() self._timers.clear() await self._message_queue.put(events.Unmount()) Reactive._reset_object(self) From a34b7c59dba7f4b7d3d2db361bdb4a1cb8284be6 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Sun, 2 Jun 2024 15:28:54 +0100 Subject: [PATCH 8/8] short circuit --- src/textual/message_pump.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/textual/message_pump.py b/src/textual/message_pump.py index ee4482bad1..b13536aeed 100644 --- a/src/textual/message_pump.py +++ b/src/textual/message_pump.py @@ -480,8 +480,9 @@ async def _close_messages(self, wait: bool = True) -> None: if self._closed or self._closing: return self._closing = True - await Timer._stop_all(self._timers) - self._timers.clear() + if self._timers: + await Timer._stop_all(self._timers) + self._timers.clear() await self._message_queue.put(events.Unmount()) Reactive._reset_object(self) await self._message_queue.put(None) @@ -518,7 +519,9 @@ async def _process_messages(self) -> None: pass finally: self._running = False - await Timer._stop_all(self._timers) + if self._timers: + await Timer._stop_all(self._timers) + self._timers.clear() async def _pre_process(self) -> bool: """Procedure to run before processing messages.