Textualize / textual

The lean application framework for Python. Build sophisticated user interfaces with a simple Python API. Run your apps in the terminal and a web browser.
https://textual.textualize.io/
MIT License
25.17k stars 772 forks source link

Awaiting pop_screen in @on causes app to freeze #5008

Open mzebrak opened 1 week ago

mzebrak commented 1 week ago

Version: 0.79.1

Adding await to self.app.pop_screen causes app to freeze when triggered via button, but works fine when triggered via key binding.

from __future__ import annotations

from typing import cast

from textual import on
from textual.app import App, ComposeResult
from textual.binding import Binding
from textual.screen import Screen
from textual.widgets import Button, Footer, Header, Label

class FirstScreen(Screen):
    def compose(self) -> ComposeResult:
        yield Header()
        yield Label("this is the first screen")
        yield Footer()

class SecondScreen(Screen):
    def compose(self) -> ComposeResult:
        yield Header()
        yield Label("this is the second screen")
        yield Button("Pop back to first screen")
        yield Footer()

    @on(Button.Pressed)
    async def pressed(self) -> None:
        app = cast(MyApp, self.app)
        app.notify("Going back to first screen...")
        await app.action_pop_current_screen()

class MyApp(App):
    BINDINGS = [
        Binding("a", "pop_current_screen", "Pop screen"),  # key binding works just fine with await
        Binding("d", "push", "Push screen"),
    ]

    async def on_mount(self) -> None:
        await self.push_screen(FirstScreen())

    async def action_pop_current_screen(self) -> None:
        while not isinstance(self.screen, FirstScreen):
            await self.pop_screen()  # awaiting causes the bug, removing await fixes it in case of pressing the button

    async def action_push(self) -> None:
        await self.push_screen(SecondScreen())

MyApp().run()
willmcgugan commented 1 week ago

By awaiting the pop_screen, you are waiting for all its messages to be processed. But your button.pressed message handler will never return, because it is waiting for itself to be popped. In other words, a deadlock.

I suspect the best thing for us to do is make it error / warning. To fix it, you can add @work to your action.

mzebrak commented 1 week ago

Putting @work over MyApp.action_pop_current_screen solves the case here. I thought it would be the same as self.run_worker(app.action_pop_current_screen()) in SecondScreen.pressed, however the second one crashes without any error.

Because real scenario is something more like: https://github.com/Textualize/textual/issues/5009 so putting @work over such an App method is a no-go for me.

By awaiting the pop_screen, you are waiting for all its messages to be processed

But in the case of pop_screen, there is no need to wait for all the screen messages to be processed I think? Because this call will destroy this screen anyway, so shouldn't it prioritize it over other messages?

mzebrak commented 6 days ago

This shows the crash I've been talking about in the previous comment. Am I doing something wrong there?

TEXTUAL CONSOLE OUTPUT ```python [08:23:50] EVENT message_pump.py:740 Key(key='enter', character='\r', name='enter', is_printable=False, aliases=['enter', 'ctrl+m']) >>> Button() method= [08:23:50] EVENT message_pump.py:740 Key(key='enter', character='\r', name='enter', is_printable=False, aliases=['enter', 'ctrl+m']) >>> SecondScreen() method= [08:23:50] EVENT message_pump.py:740 Key(key='enter', character='\r', name='enter', is_printable=False, aliases=['enter', 'ctrl+m']) >>> MyApp(title='MyApp', classes={'-dark-mode'}) method= [08:23:50] SYSTEM app.py:3444 namespace=Button() action_name='press' params=() [08:23:50] EVENT message_pump.py:749 Pressed() >>> Button() method=None [08:23:50] EVENT message_pump.py:740 Pressed() >>> SecondScreen() method= [08:23:50] EVENT message_pump.py:749 StateChanged(, ) >>> SecondScreen() method=None [08:23:50] EVENT message_pump.py:749 StateChanged(, ) >>> SecondScreen() method=None [08:23:50] EVENT message_pump.py:740 Notify(notification=Notification(message='Going back to first screen...', title='', severity='information', timeout=5, raised_at=1727072630.3777125, identity='e34262f4-1c0b-4ec1-b2ad-79a4b57ab4b4')) >>> MyApp(title='MyApp', classes={'-dark-mode'}) method= [08:23:50] EVENT message_pump.py:749 Pressed() >>> MyApp(title='MyApp', classes={'-dark-mode'}) method=None [08:23:50] WORKER worker.py:366 [08:23:50] SYSTEM app.py:2494 FirstScreen() is active [08:23:50] EVENT message_pump.py:740 ScreenResume() >>> FirstScreen() method= [08:23:50] SYSTEM app.py:2272 SecondScreen() SUSPENDED [08:23:50] DEBUG screen.py:827 focus was removed [08:23:50] EVENT message_pump.py:740 Mount() >>> Toast() method= [08:23:50] EVENT message_pump.py:740 Mount() >>> Toast() method= [08:23:50] EVENT message_pump.py:740 ScreenSuspend() >>> SecondScreen() method= [08:23:50] EVENT message_pump.py:740 Blur() >>> Button() method= [08:23:50] EVENT message_pump.py:740 Mount() >>> Toast() method= [08:23:50] EVENT message_pump.py:740 Mount() >>> Toast() method= [08:23:50] EVENT message_pump.py:740 Mount() >>> ToastHolder(id='--textual-toast-e34262f4-1c0b-4ec1-b2ad-79a4b57ab4b4') method= [08:23:50] EVENT message_pump.py:740 Unmount() >>> Tooltip(id='textual-tooltip') method= [08:23:50] EVENT message_pump.py:740 Unmount() >>> Label() method= [08:23:50] EVENT message_pump.py:740 Mount() >>> ToastHolder(id='--textual-toast-e34262f4-1c0b-4ec1-b2ad-79a4b57ab4b4') method= [08:23:50] EVENT message_pump.py:740 Unmount() >>> HeaderIcon() method= [08:23:50] EVENT message_pump.py:740 Unmount() >>> HeaderTitle() method= [08:23:50] EVENT message_pump.py:740 Unmount() >>> HeaderClockSpace() method= [08:23:50] EVENT message_pump.py:740 Unmount() >>> FooterKey() method= [08:23:50] EVENT message_pump.py:740 Unmount() >>> FooterKey() method= [08:23:50] EVENT message_pump.py:740 Unmount() >>> FooterKey() method= [08:23:50] EVENT message_pump.py:740 Unmount() >>> Header() method= [08:23:50] EVENT message_pump.py:740 Unmount() >>> Footer() method= [08:23:50] EVENT message_pump.py:740 Unmount() >>> Footer() method= [08:23:50] EVENT message_pump.py:740 Unmount() >>> Button() method= [08:23:50] EVENT message_pump.py:740 Unmount() >>> Toast() method= [08:23:50] EVENT message_pump.py:740 Unmount() >>> ToastHolder(id='--textual-toast-e34262f4-1c0b-4ec1-b2ad-79a4b57ab4b4') method= [08:23:50] EVENT message_pump.py:740 Unmount() >>> ToastRack(id='textual-toastrack') method= [08:23:50] EVENT message_pump.py:740 Unmount() >>> SecondScreen() method= [08:23:50] WORKER worker.py:372 [08:23:50] EVENT message_pump.py:740 Unmount() >>> Tooltip(id='textual-tooltip') method= [08:23:50] EVENT message_pump.py:740 Unmount() >>> Label() method= [08:23:50] EVENT message_pump.py:740 Unmount() >>> HeaderIcon() method= [08:23:50] EVENT message_pump.py:740 Unmount() >>> HeaderTitle() method= [08:23:50] EVENT message_pump.py:740 Unmount() >>> HeaderClockSpace() method= [08:23:50] EVENT message_pump.py:740 Unmount() >>> FooterKey() method= [08:23:50] EVENT message_pump.py:740 Unmount() >>> FooterKey() method= [08:23:50] EVENT message_pump.py:740 Unmount() >>> FooterKey() method= [08:23:50] EVENT message_pump.py:740 Unmount() >>> Header() method= [08:23:50] EVENT message_pump.py:740 Unmount() >>> Footer() method= [08:23:50] EVENT message_pump.py:740 Unmount() >>> Footer() method= [08:23:50] EVENT message_pump.py:740 Unmount() >>> Toast() method= [08:23:50] EVENT message_pump.py:740 Unmount() >>> ToastHolder(id='--textual-toast-e34262f4-1c0b-4ec1-b2ad-79a4b57ab4b4') method= [08:23:50] EVENT message_pump.py:740 Unmount() >>> ToastRack(id='textual-toastrack') method= [08:23:50] EVENT message_pump.py:740 Unmount() >>> FirstScreen() method= [08:23:50] EVENT message_pump.py:740 Unmount() >>> ToastRack(id='textual-toastrack') method= [08:23:50] EVENT message_pump.py:740 Unmount() >>> Tooltip(id='textual-tooltip') method= [08:23:50] EVENT message_pump.py:740 Unmount() >>> Screen(id='_default') method= ───────────────────────────────────────────────────────────────────────────────────────────── Client '127.0.0.1' disconnected ───────────────────────────────────────────────────────────────────────────────────────────── [08:23:50] EVENT message_pump.py:749 Unmount() >>> MyApp(title='MyApp', classes={'-dark-mode'}) method=None ```
from __future__ import annotations

from typing import cast

from textual import on
from textual.app import App, ComposeResult
from textual.binding import Binding
from textual.screen import Screen
from textual.widgets import Button, Footer, Header, Label

class FirstScreen(Screen):
    def compose(self) -> ComposeResult:
        yield Header()
        yield Label("this is the first screen")
        yield Footer()

class SecondScreen(Screen):
    def compose(self) -> ComposeResult:
        yield Header()
        yield Label("this is the second screen")
        yield Button("Pop back to first screen")
        yield Footer()

    @on(Button.Pressed)
    async def pressed(self) -> None:
        app = cast(MyApp, self.app)
        app.notify("Going back to first screen...")
        self.run_worker(app.action_pop_current_screen())  # causes app closure

class MyApp(App):
    BINDINGS = [
        Binding("a", "pop_current_screen", "Pop screen"),  # key binding works just fine with await
        Binding("d", "push", "Push screen"),
    ]

    async def on_mount(self) -> None:
        await self.push_screen(FirstScreen())

    async def action_pop_current_screen(self) -> None:
        while not isinstance(self.screen, FirstScreen):
            await self.pop_screen()  # awaiting causes the bug, removing await fixes it in case of pressing the button

    async def action_push(self) -> None:
        await self.push_screen(SecondScreen())

MyApp().run()
mzebrak commented 2 days ago

I investigated a bit and found out that when the app closure happens, there is asyncio.CancelledError being raised from this line: https://github.com/Textualize/textual/blob/main/src/textual/worker.py#L339

After adding a wrapper to log cancellation inspired by https://stackoverflow.com/a/71356489 and some more logging looks like it comes from await_prune of AwaitRemove:

272 │ 2024-09-26 10:35:07.036 | ❌ ERROR    | textual.worker:_run:380 - In worker run of: action_pop_current_screen
 273 │ 2024-09-26 10:35:07.037 | ❌ ERROR    | textual.worker:_run_async:338 - In _run_async of: action_pop_current_screen
 274 │ 2024-09-26 10:35:07.037 | ❌ ERROR    | textual.worker:_run_async:339 - self._work is <coroutine object MyApp.action_pop_current_screen at 0x7f13aa1a78b0>
 275 │ 2024-09-26 10:35:07.037 | ❌ ERROR    | textual.worker:_run_async:348 - awaiting self._work
 276 │ 2024-09-26 10:35:07.064 | ❌ ERROR    | textual.utils:wrapper:27 - Cancelled <function sleep at 0x7f13aefa4e50>
 277 │ 2024-09-26 10:35:07.064 | ❌ ERROR    | textual.utils:wrapper:27 - Cancelled <function Timer._run at 0x7f13aee51fc0>
 278 │ 2024-09-26 10:35:07.064 | ❌ ERROR    | textual.utils:wrapper:27 - Cancelled <function sleep at 0x7f13aefa4e50>
 279 │ 2024-09-26 10:35:07.065 | ❌ ERROR    | textual.utils:wrapper:27 - Cancelled <function Timer._run at 0x7f13aee51fc0>
 280 │ 2024-09-26 10:35:07.067 | ❌ ERROR    | textual.utils:wrapper:27 - Cancelled <function sleep at 0x7f13aefa4e50>
 281 │ 2024-09-26 10:35:07.068 | ❌ ERROR    | textual.utils:wrapper:27 - Cancelled <function Timer._run at 0x7f13aee51fc0>
 282 │ 2024-09-26 10:35:07.069 | ❌ ERROR    | textual.utils:wrapper:27 - Cancelled <function sleep at 0x7f13aefa4e50>
 283 │ 2024-09-26 10:35:07.069 | ❌ ERROR    | textual.utils:wrapper:27 - Cancelled <function Timer._run at 0x7f13aee51fc0>
 284 │ 2024-09-26 10:35:07.070 | ❌ ERROR    | textual.utils:wrapper:27 - Cancelled <function AwaitRemove.__await__.<locals>.await_prune at 0x7f13a9fe1360>
 285 │ 2024-09-26 10:35:07.070 | ❌ ERROR    | textual.worker:_run_async:352 - CancelledError: CancelledError()

further narrowing shows it comes from await gather(*tasks) and we can see there is <Task cancelled name='message pump SecondScreen()', ...> there

mzebrak commented 1 day ago

Created a separate issue for the self.run_worker as seems like this is a different thing than the original cause of this issue which was awaiting pop_screen in @on. See: https://github.com/Textualize/textual/issues/5064

matmaer commented 1 day ago

Adding await to self.app.pop_screen causes app to freeze when triggered via button, but works fine when triggered via key binding.

Check inheritage of classes, you are using BINDINGS. There's a lot going on when triggered, compare the objects. Hope this helps.

mzebrak commented 1 day ago

Adding await to self.app.pop_screen causes app to freeze when triggered via button, but works fine when triggered via key binding.

Check inheritage of classes, you are using BINDINGS. There's a lot going on when triggered, compare the objects. Hope this helps.

It is explained why it happens, see Will's answer above. Workaround of running in a background worker can be used, but still, I'm not sure it should work like that.

It's just weird the bindings action is run in a safe place, and the button action/event is not, while they're defined on the same screen.

In my case it occurred in code like this:

@on(Button.pressed)
async def button_finish(self) -> None:
    await self._finish()

async def action_finish(self) -> None:
    await self._finish()

Very hard to get the difference from the screen's POV. (action_finish works just OK, while button_finish does not)

Workaround is:

@on(Button.pressed)
async def button_finish(self) -> None:
    self.app.run_worker(self._finish())  # has to be self.app.run_worker, not self.run_worker in some cases, see: #5064

async def action_finish(self) -> None:
    await self._finish()