From f07684438f1d4f9f18022b1e0d21b908ca299a23 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Fri, 18 Nov 2022 11:34:52 +0000 Subject: [PATCH 1/4] fix remove freeze --- src/textual/app.py | 38 ++++++++++++++++++++++++++++++++++++++ src/textual/css/query.py | 11 ++--------- src/textual/events.py | 22 ---------------------- src/textual/screen.py | 23 ++++++++++++----------- src/textual/widget.py | 23 ++++++++++------------- 5 files changed, 62 insertions(+), 55 deletions(-) diff --git a/src/textual/app.py b/src/textual/app.py index e74a90f8a..e69a82214 100644 --- a/src/textual/app.py +++ b/src/textual/app.py @@ -39,6 +39,7 @@ from rich.traceback import Traceback from . import Logger, LogGroup, LogVerbosity, actions, events, log, messages from ._animator import DEFAULT_EASING, Animatable, Animator, EasingFunction +from .await_remove import AwaitRemove from ._ansi_sequences import SYNC_END, SYNC_START from ._callback import invoke from ._context import active_app @@ -353,6 +354,7 @@ class App(Generic[ReturnType], DOMNode): else None ) self._screenshot: str | None = None + self._dom_lock = asyncio.Lock() @property def return_value(self) -> ReturnType | None: @@ -1936,6 +1938,42 @@ class App(Generic[ReturnType], DOMNode): for child in widget.children: push(child) + def _remove_nodes(self, widgets: list[Widget]) -> AwaitRemove: + """Remove nodes from DOM, and return an awaitable that awaits cleanup. + + Args: + widgets (list[Widget]): List of nodes to remvoe. + + Returns: + AwaitRemove: Awaitable that returns when the nodes have been fully removed. + """ + + async def remove_task( + widgets: list[Widget], finished_event: asyncio.Event + ) -> None: + try: + await self._prune_nodes(widgets) + finally: + finished_event.set() + + removed_widgets = self._detach_from_dom(widgets) + self.refresh(layout=True) + + finished_event = asyncio.Event() + asyncio.create_task(remove_task(removed_widgets, finished_event)) + + return AwaitRemove(finished_event) + + async def _prune_nodes(self, widgets: list[Widget]) -> None: + """Remove nodes and children. + + Args: + widgets (Widget): _description_ + """ + async with self._dom_lock: + for widget in widgets: + 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/css/query.py b/src/textual/css/query.py index ff1657a99..19f999e48 100644 --- a/src/textual/css/query.py +++ b/src/textual/css/query.py @@ -356,16 +356,9 @@ class DOMQuery(Generic[QueryType]): Returns: AwaitRemove: An awaitable object that waits for the widgets to be removed. """ - prune_finished_event = asyncio.Event() app = active_app.get() - app.post_message_no_wait( - events.Prune( - app, - widgets=app._detach_from_dom(list(self)), - finished_flag=prune_finished_event, - ) - ) - return AwaitRemove(prune_finished_event) + await_remove = app._remove_nodes(list(self)) + return await_remove def set_styles( self, css: str | None = None, **update_styles diff --git a/src/textual/events.py b/src/textual/events.py index 0eb5abac3..815f75f02 100644 --- a/src/textual/events.py +++ b/src/textual/events.py @@ -127,28 +127,6 @@ class Unmount(Mount, bubble=False, verbose=False): """Sent when a widget is unmounted and may not longer receive messages.""" -class Prune(Event, bubble=False): - """Sent to the app to ask it to prune one or more widgets from the DOM. - - Attributes: - widgets (list[Widgets]): The list of widgets to prune. - finished_flag (asyncio.Event): An asyncio Event to that will be flagged when the prune is done. - """ - - def __init__( - self, sender: MessageTarget, widgets: list[Widget], finished_flag: asyncio.Event - ) -> None: - """Initialise the event. - - Args: - widgets (list[Widgets]): The list of widgets to prune. - finished_flag (asyncio.Event): An asyncio Event to that will be flagged when the prune is done. - """ - super().__init__(sender) - self.finished_flag = finished_flag - self.widgets = widgets - - class Show(Event, bubble=False): """Sent when a widget has become visible.""" diff --git a/src/textual/screen.py b/src/textual/screen.py index 0069c5a79..2fd22308a 100644 --- a/src/textual/screen.py +++ b/src/textual/screen.py @@ -310,18 +310,19 @@ class Screen(Widget): # Check for any widgets marked as 'dirty' (needs a repaint) event.prevent_default() - if self.is_current: - if self._layout_required: - self._refresh_layout() - self._layout_required = False - self._dirty_widgets.clear() - if self._repaint_required: - self._dirty_widgets.clear() - self._dirty_widgets.add(self) - self._repaint_required = False + async with self.app._dom_lock: + if self.is_current: + if self._layout_required: + self._refresh_layout() + self._layout_required = False + self._dirty_widgets.clear() + if self._repaint_required: + self._dirty_widgets.clear() + self._dirty_widgets.add(self) + self._repaint_required = False - if self._dirty_widgets: - self.update_timer.resume() + if self._dirty_widgets: + self.update_timer.resume() # The Screen is idle - a good opportunity to invoke the scheduled callbacks await self._invoke_and_clear_callbacks() diff --git a/src/textual/widget.py b/src/textual/widget.py index 8b8cc0fd6..fcc8aa328 100644 --- a/src/textual/widget.py +++ b/src/textual/widget.py @@ -523,15 +523,19 @@ class Widget(DOMNode): # Decide the final resting place depending on what we've been asked # to do. + insert_before: int | None = None + insert_after: int | None = None if before is not None: - parent, before = self._find_mount_point(before) + parent, insert_before = self._find_mount_point(before) elif after is not None: - parent, after = self._find_mount_point(after) + parent, insert_after = self._find_mount_point(after) else: parent = self return AwaitMount( - self.app._register(parent, *widgets, before=before, after=after) + self.app._register( + parent, *widgets, before=insert_before, after=insert_after + ) ) def move_child( @@ -697,7 +701,6 @@ class Widget(DOMNode): Returns: int: The height of the content. """ - if self.is_container: assert self._layout is not None height = ( @@ -2114,15 +2117,9 @@ class Widget(DOMNode): Returns: AwaitRemove: An awaitable object that waits for the widget to be removed. """ - prune_finished_event = AsyncEvent() - self.app.post_message_no_wait( - events.Prune( - self, - widgets=self.app._detach_from_dom([self]), - finished_flag=prune_finished_event, - ) - ) - return AwaitRemove(prune_finished_event) + + await_remove = self.app._remove_nodes([self]) + return await_remove def render(self) -> RenderableType: """Get renderable for widget. From ac1c6a2f3c2830d95e22c4fcf2c3b15106e69ebd Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Fri, 18 Nov 2022 11:39:35 +0000 Subject: [PATCH 2/4] rename --- src/textual/app.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/textual/app.py b/src/textual/app.py index e69a82214..1961f1f47 100644 --- a/src/textual/app.py +++ b/src/textual/app.py @@ -18,6 +18,7 @@ from time import perf_counter from typing import ( TYPE_CHECKING, Any, + Callable, Generic, Iterable, List, @@ -25,7 +26,6 @@ from typing import ( TypeVar, Union, cast, - Callable, ) from weakref import WeakSet, WeakValueDictionary @@ -39,14 +39,14 @@ from rich.traceback import Traceback from . import Logger, LogGroup, LogVerbosity, actions, events, log, messages from ._animator import DEFAULT_EASING, Animatable, Animator, EasingFunction -from .await_remove import AwaitRemove from ._ansi_sequences import SYNC_END, SYNC_START from ._callback import invoke from ._context import active_app from ._event_broker import NoHandler, extract_handler_actions from ._filter import LineFilter, Monochrome from ._path import _make_path_object_relative -from ._typing import TypeAlias, Final +from ._typing import Final, TypeAlias +from .await_remove import AwaitRemove from .binding import Binding, Bindings from .css.query import NoMatches from .css.stylesheet import Stylesheet @@ -62,7 +62,7 @@ from .messages import CallbackType from .reactive import Reactive from .renderables.blank import Blank from .screen import Screen -from .widget import AwaitMount, Widget, MountError +from .widget import AwaitMount, MountError, Widget if TYPE_CHECKING: from .devtools.client import DevtoolsClient @@ -1948,9 +1948,15 @@ class App(Generic[ReturnType], DOMNode): AwaitRemove: Awaitable that returns when the nodes have been fully removed. """ - async def remove_task( + async def prune_widgets_task( widgets: list[Widget], finished_event: asyncio.Event ) -> None: + """Prune widgets as a background task. + + Args: + widgets (list[Widget]): Widgets to prune. + finished_event (asyncio.Event): Event to set when complete. + """ try: await self._prune_nodes(widgets) finally: @@ -1960,7 +1966,7 @@ class App(Generic[ReturnType], DOMNode): self.refresh(layout=True) finished_event = asyncio.Event() - asyncio.create_task(remove_task(removed_widgets, finished_event)) + asyncio.create_task(prune_widgets_task(removed_widgets, finished_event)) return AwaitRemove(finished_event) From d4d517aa8f94fd07e633a0f0811dab9284dedea2 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Fri, 18 Nov 2022 11:40:58 +0000 Subject: [PATCH 3/4] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f95e4d10..312b85941 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Fixed containers with transparent background not showing borders https://github.com/Textualize/textual/issues/1175 - Fixed auto-width in horizontal containers https://github.com/Textualize/textual/pull/1155 - Fixed Input cursor invisible when placeholder empty https://github.com/Textualize/textual/pull/1202 +- Fixed deadlock when remove widgets from the App https://github.com/Textualize/textual/pull/1219 ## [0.4.0] - 2022-11-08 From d90a1ea69101378b9604feed64acb4b6f3faee74 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Fri, 18 Nov 2022 12:30:26 +0000 Subject: [PATCH 4/4] words --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 312b85941..4f4579962 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,7 +41,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Fixed containers with transparent background not showing borders https://github.com/Textualize/textual/issues/1175 - Fixed auto-width in horizontal containers https://github.com/Textualize/textual/pull/1155 - Fixed Input cursor invisible when placeholder empty https://github.com/Textualize/textual/pull/1202 -- Fixed deadlock when remove widgets from the App https://github.com/Textualize/textual/pull/1219 +- Fixed deadlock when removing widgets from the App https://github.com/Textualize/textual/pull/1219 ## [0.4.0] - 2022-11-08