Skip to content

Commit

Permalink
Merge pull request #4575 from Textualize/timer-close
Browse files Browse the repository at this point in the history
Fix error on shutdown
  • Loading branch information
willmcgugan authored Jun 2, 2024
2 parents 7866974 + a34b7c5 commit 3f3e11e
Show file tree
Hide file tree
Showing 7 changed files with 65 additions and 25 deletions.
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
- Fixed Footer flicker on initial focus https://github.com/Textualize/textual/issues/4573

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

0 comments on commit 3f3e11e

Please sign in to comment.