Merge pull request #1219 from Textualize/remove-freeze-fix

fix remove freeze

Author: willmcgugan

CHANGELOG.md

@@ -45,6 +45,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 removing widgets from the App https://github.com/Textualize/textual/pull/1219
 
 ## [0.4.0] - 2022-11-08
 

src/textual/app.py

@@ -18,14 +18,14 @@
 from typing import (
     TYPE_CHECKING,
     Any,
+    Callable,
     Generic,
     Iterable,
     List,
     Type,
     TypeVar,
     Union,
     cast,
-    Callable,
 )
 from weakref import WeakSet, WeakValueDictionary
 
@@ -45,7 +45,8 @@
 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
@@ -61,7 +62,8 @@
 from .reactive import Reactive
 from .renderables.blank import Blank
 from .screen import Screen
-from .widget import AwaitMount, Widget
+from .widget import AwaitMount, MountError, Widget
+
 
 if TYPE_CHECKING:
     from .devtools.client import DevtoolsClient
@@ -352,6 +354,7 @@ def __init__(
             else None
         )
         self._screenshot: str | None = None
+        self._dom_lock = asyncio.Lock()
 
     @property
     def return_value(self) -> ReturnType | None:
@@ -1951,6 +1954,48 @@ def _walk_children(self, root: Widget) -> Iterable[list[Widget]]:
             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 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:
+                finished_event.set()
+
+        removed_widgets = self._detach_from_dom(widgets)
+        self.refresh(layout=True)
+
+        finished_event = asyncio.Event()
+        asyncio.create_task(prune_widgets_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.
 

src/textual/css/query.py

@@ -356,16 +356,9 @@ def remove(self) -> AwaitRemove:
         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

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."""
 

src/textual/screen.py

@@ -310,18 +310,19 @@ async def _on_idle(self, event: events.Idle) -> None:
         # 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
-
-            if self._dirty_widgets:
-                self.update_timer.resume()
+        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()
 
         # The Screen is idle - a good opportunity to invoke the scheduled callbacks
         await self._invoke_and_clear_callbacks()

src/textual/widget.py

@@ -523,15 +523,19 @@ def mount(
 
         # 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 @@ def get_content_height(self, container: Size, viewport: Size, width: int) -> int
         Returns:
             int: The height of the content.
         """
-
         if self.is_container:
             assert self._layout is not None
             height = (
@@ -2114,15 +2117,9 @@ def remove(self) -> AwaitRemove:
         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.