Skip to content

Commit

Permalink
Merge pull request #4615 from Textualize/fix-notify-error
Browse files Browse the repository at this point in the history
Fix notifications crash
  • Loading branch information
willmcgugan authored Jun 6, 2024
2 parents 7925273 + d45d19b commit 1613d7c
Show file tree
Hide file tree
Showing 12 changed files with 163 additions and 29 deletions.
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion examples/dictionary.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
@@ -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/"
Expand Down
2 changes: 1 addition & 1 deletion src/textual/_callback.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
45 changes: 25 additions & 20 deletions src/textual/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/textual/message_pump.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
61 changes: 61 additions & 0 deletions src/textual/rlock.py
Original file line number Diff line number Diff line change
@@ -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())
6 changes: 3 additions & 3 deletions src/textual/widget.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
2 changes: 2 additions & 0 deletions src/textual/widgets/_markdown.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 0 additions & 1 deletion src/textual/widgets/_toast.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
56 changes: 56 additions & 0 deletions tests/test_rlock.py
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion tests/test_widget_removing.py
Original file line number Diff line number Diff line change
Expand Up @@ -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] = []
Expand Down

0 comments on commit 1613d7c

Please sign in to comment.