Fix notifications crash

CHANGELOG.md

@@ -5,6 +5,16 @@ 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/).
 
+## [0.65.2] - 2023-06-06
+
+### Fixed
+
+- Fixed issue with notifications and screen switches https://github.com/Textualize/textual/pull/4615
+
+### Added
+
+- Added textual.rlock.RLock https://github.com/Textualize/textual/pull/4615
+
 ## [0.65.1] - 2024-06-05
 
 ### Fixed
@@ -2069,6 +2079,7 @@ https://textual.textualize.io/blog/2022/11/08/version-040/#version-040
 - New handler system for messages that doesn't require inheritance
 - Improved traceback handling
 
+[0.65.2]: https://github.com/Textualize/textual/compare/v0.65.1...v0.65.2
 [0.65.1]: https://github.com/Textualize/textual/compare/v0.65.0...v0.65.1
 [0.65.0]: https://github.com/Textualize/textual/compare/v0.64.0...v0.65.0
 [0.64.0]: https://github.com/Textualize/textual/compare/v0.63.6...v0.64.0

examples/dictionary.py

@@ -33,7 +33,7 @@ async def on_input_changed(self, message: Input.Changed) -> None:
             self.lookup_word(message.value)
         else:
             # Clear the results
-            self.query_one("#results", Markdown).update("")
+            await self.query_one("#results", Markdown).update("")
 
     @work(exclusive=True)
     async def lookup_word(self, word: str) -> None:

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "textual"
-version = "0.65.1"
+version = "0.65.2"
 homepage = "https://github.com/Textualize/textual"
 repository = "https://github.com/Textualize/textual"
 documentation = "https://textual.textualize.io/"

src/textual/_callback.py

@@ -48,7 +48,7 @@ async def _invoke(callback: Callable, *params: object) -> Any:
     return result
 
 
-async def invoke(callback: Callable[[], Any], *params: object) -> Any:
+async def invoke(callback: Callable[..., Any], *params: object) -> Any:
     """Invoke a callback with an arbitrary number of parameters.
 
     Args:

src/textual/app.py

@@ -103,6 +103,7 @@
 from .notifications import Notification, Notifications, Notify, SeverityLevel
 from .reactive import Reactive
 from .renderables.blank import Blank
+from .rlock import RLock
 from .screen import (
     ActiveBinding,
     Screen,
@@ -579,7 +580,7 @@ def __init__(
             else None
         )
         self._screenshot: str | None = None
-        self._dom_lock = asyncio.Lock()
+        self._dom_lock = RLock()
         self._dom_ready = False
         self._batch_count = 0
         self._notifications = Notifications()
@@ -2751,23 +2752,24 @@ def is_mounted(self, widget: Widget) -> bool:
     async def _close_all(self) -> None:
         """Close all message pumps."""
 
-        # Close all screens on all stacks:
-        for stack in self._screen_stacks.values():
-            for stack_screen in reversed(stack):
-                if stack_screen._running:
-                    await self._prune_node(stack_screen)
-            stack.clear()
-
-        # Close pre-defined screens.
-        for screen in self.SCREENS.values():
-            if isinstance(screen, Screen) and screen._running:
-                await self._prune_node(screen)
-
-        # Close any remaining nodes
-        # Should be empty by now
-        remaining_nodes = list(self._registry)
-        for child in remaining_nodes:
-            await child._close_messages()
+        async with self._dom_lock:
+            # Close all screens on all stacks:
+            for stack in self._screen_stacks.values():
+                for stack_screen in reversed(stack):
+                    if stack_screen._running:
+                        await self._prune_node(stack_screen)
+                stack.clear()
+
+            # Close pre-defined screens.
+            for screen in self.SCREENS.values():
+                if isinstance(screen, Screen) and screen._running:
+                    await self._prune_node(screen)
+
+            # Close any remaining nodes
+            # Should be empty by now
+            remaining_nodes = list(self._registry)
+            for child in remaining_nodes:
+                await child._close_messages()
 
     async def _shutdown(self) -> None:
         self._begin_batch()  # Prevents any layout / repaint while shutting down
@@ -3341,7 +3343,10 @@ async def prune_widgets_task(
                 await self._prune_nodes(widgets)
             finally:
                 finished_event.set()
-                self._update_mouse_over(self.screen)
+                try:
+                    self._update_mouse_over(self.screen)
+                except ScreenStackError:
+                    pass
                 if parent is not None:
                     parent.refresh(layout=True)
 
@@ -3555,7 +3560,7 @@ def _refresh_notifications(self) -> None:
                 # or one will turn up. Things will work out later.
                 return
             # Update the toast rack.
-            toast_rack.show(self._notifications)
+            self.call_later(toast_rack.show, self._notifications)
 
     def notify(
         self,

src/textual/message_pump.py

@@ -746,7 +746,7 @@ async def _on_message(self, message: Message) -> None:
             if message._sender is not None and message._sender == self._parent:
                 # parent is sender, so we stop propagation after parent
                 message.stop()
-            if self.is_parent_active and not self._parent._closing:
+            if self.is_parent_active and self.is_attached:
                 message._bubble_to(self._parent)
 
     def check_idle(self) -> None:

src/textual/rlock.py

@@ -0,0 +1,61 @@
+from __future__ import annotations
+
+from asyncio import Lock, Task, current_task
+
+
+class RLock:
+    """A re-entrant asyncio lock."""
+
+    def __init__(self) -> None:
+        self._owner: Task | None = None
+        self._count = 0
+        self._lock = Lock()
+
+    async def acquire(self) -> None:
+        """Wait until the lock can be acquired."""
+        task = current_task()
+        assert task is not None
+        if self._owner is None or self._owner is not task:
+            await self._lock.acquire()
+            self._owner = task
+        self._count += 1
+
+    def release(self) -> None:
+        """Release a previously acquired lock."""
+        task = current_task()
+        assert task is not None
+        self._count -= 1
+        if self._count < 0:
+            # Should not occur if every acquire as a release
+            raise RuntimeError("RLock.release called too many times")
+        if self._owner is task:
+            if not self._count:
+                self._owner = None
+                self._lock.release()
+
+    @property
+    def is_locked(self):
+        """Return True if lock is acquired."""
+        return self._lock.locked()
+
+    async def __aenter__(self) -> None:
+        """Asynchronous context manager to acquire and release lock."""
+        await self.acquire()
+
+    async def __aexit__(self, _type, _value, _traceback) -> None:
+        """Exit the context manager."""
+        self.release()
+
+
+if __name__ == "__main__":
+    from asyncio import Lock
+
+    async def locks():
+        lock = RLock()
+        async with lock:
+            async with lock:
+                print("Hello")
+
+    import asyncio
+
+    asyncio.run(locks())

src/textual/widget.py

@@ -4,7 +4,7 @@
 
 from __future__ import annotations
 
-from asyncio import Lock, create_task, wait
+from asyncio import create_task, wait
 from collections import Counter
 from contextlib import asynccontextmanager
 from fractions import Fraction
@@ -81,6 +81,7 @@
 from .reactive import Reactive
 from .render import measure
 from .renderables.blank import Blank
+from .rlock import RLock
 from .strip import Strip
 from .walk import walk_depth_first
 
@@ -396,7 +397,7 @@ def __init__(
         if self.BORDER_SUBTITLE:
             self.border_subtitle = self.BORDER_SUBTITLE
 
-        self.lock = Lock()
+        self.lock = RLock()
         """`asyncio` lock to be used to synchronize the state of the widget.
 
         Two different tasks might call methods on a widget at the same time, which
@@ -3550,7 +3551,6 @@ def post_message(self, message: Message) -> bool:
                 self.log.warning(self, f"IS NOT RUNNING, {message!r} not sent")
             except NoActiveAppError:
                 pass
-
         return super().post_message(message)
 
     async def _on_idle(self, event: events.Idle) -> None:

src/textual/widgets/_markdown.py

@@ -1012,6 +1012,8 @@ async def mount_batch(batch: list[MarkdownBlock]) -> None:
                         batch.clear()
                 if batch:
                     await mount_batch(batch)
+                if not removed:
+                    await markdown_block.remove()
 
             self._table_of_contents = table_of_contents
 

src/textual/widgets/_toast.py

@@ -183,7 +183,6 @@ def show(self, notifications: Notifications) -> None:
         Args:
             notifications: The notifications to show.
         """
-
         # Look for any stale toasts and remove them.
         for toast in self.query(Toast):
             if toast._notification not in notifications:

tests/test_rlock.py

@@ -0,0 +1,56 @@
+import asyncio
+
+import pytest
+
+from textual.rlock import RLock
+
+
+async def test_simple_lock():
+    lock = RLock()
+    # Starts not locked
+    assert not lock.is_locked
+    # Acquire the lock
+    await lock.acquire()
+    assert lock.is_locked
+    # Acquire a second time (should not block)
+    await lock.acquire()
+    assert lock.is_locked
+
+    # Release the lock
+    lock.release()
+    # Should still be locked
+    assert lock.is_locked
+    # Release the lock
+    lock.release()
+    # Should be released
+    assert not lock.is_locked
+
+    # Another release is a runtime error
+    with pytest.raises(RuntimeError):
+        lock.release()
+
+
+async def test_multiple_tasks() -> None:
+    """Check RLock prevents other tasks from acquiring lock."""
+    lock = RLock()
+
+    started: list[int] = []
+    done: list[int] = []
+
+    async def test_task(n: int) -> None:
+        started.append(n)
+        async with lock:
+            done.append(n)
+
+    async with lock:
+        assert done == []
+        task1 = asyncio.create_task(test_task(1))
+        assert sorted(started) == []
+        task2 = asyncio.create_task(test_task(2))
+        await asyncio.sleep(0)
+        assert sorted(started) == [1, 2]
+
+    await task1
+    assert 1 in done
+    await task2
+    assert 2 in done

tests/test_widget_removing.py

@@ -78,7 +78,7 @@ async def test_remove_move_focus():
         assert pilot.app.focused == buttons[9]
 
 
-async def test_widget_remove_order():
+async def test_widget_remove_order() -> None:
     """A Widget.remove of a top-level widget should cause bottom-first removal."""
 
     removals: list[str] = []