Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix error on shutdown #4575

Merged
merged 9 commits into from
Jun 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

### Fixed

- Fix traceback on exit https://github.com/Textualize/textual/pull/4575
- Fixed `Markdown.goto_anchor` no longer scrolling the heading into view https://github.com/Textualize/textual/pull/4583

## [0.63.6] - 2024-05-29
Expand Down
5 changes: 3 additions & 2 deletions src/textual/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -3373,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

Expand Down
17 changes: 10 additions & 7 deletions src/textual/message_pump.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -479,10 +480,9 @@ 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()
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)
Expand Down Expand Up @@ -519,8 +519,9 @@ async def _process_messages(self) -> None:
pass
finally:
self._running = False
for timer in list(self._timers):
timer.stop()
if self._timers:
await Timer._stop_all(self._timers)
self._timers.clear()

async def _pre_process(self) -> bool:
"""Procedure to run before processing messages.
Expand Down Expand Up @@ -849,6 +850,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:
Expand Down
1 change: 1 addition & 0 deletions src/textual/reactive.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ def invoke_watcher(
value: The new value of the attribute.
"""
_rich_traceback_omit = True

param_count = count_parameters(watch_function)
reset_token = active_message_pump.set(watcher_object)
try:
Expand Down
2 changes: 1 addition & 1 deletion src/textual/signal.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
48 changes: 40 additions & 8 deletions src/textual/timer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -83,12 +83,44 @@ 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:

async def noop() -> None:
"""A dummy task."""

return create_task(noop())

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: 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()
await timer._task

await gather(*[stop_timer(timer) for timer in list(timers)])

def pause(self) -> None:
"""Pause the timer.
Expand Down
16 changes: 9 additions & 7 deletions src/textual/widget.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -3728,7 +3729,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.
Expand Down
Loading