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

Awaiting pop_screen in @on causes app to freeze #5008

Closed
mzebrak opened this issue Sep 16, 2024 · 8 comments · Fixed by #5069
Closed

Awaiting pop_screen in @on causes app to freeze #5008

mzebrak opened this issue Sep 16, 2024 · 8 comments · Fixed by #5069

Comments

@mzebrak
Copy link

mzebrak commented Sep 16, 2024

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()
@Textualize Textualize deleted a comment from github-actions bot Sep 16, 2024
@willmcgugan
Copy link
Collaborator

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
Copy link
Author

mzebrak commented Sep 16, 2024

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: #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
Copy link
Author

mzebrak commented Sep 23, 2024

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

TEXTUAL CONSOLE OUTPUT
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Key(key='enter', character='\r', name='enter', is_printable=False, aliases=['enter', 'ctrl+m']) >>> Button() method=<Widget.on_key>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Key(key='enter', character='\r', name='enter', is_printable=False, aliases=['enter', 'ctrl+m']) >>> SecondScreen() method=<Widget.on_key>
[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=<App.on_key>
[08:23:50] SYSTEM                                                                                                                                                                                               app.py:3444
<action> 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=<SecondScreen.on_button_pressed>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:749
StateChanged(<Worker RUNNING name='action_pop_current_screen' description='<coroutine object MyApp.action_pop_current_screen at 0x7f99c3f2f0d0>'>, <WorkerState.PENDING: 1>) >>> SecondScreen() method=None
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:749
StateChanged(<Worker RUNNING name='action_pop_current_screen' description='<coroutine object MyApp.action_pop_current_screen at 0x7f99c3f2f0d0>'>, <WorkerState.RUNNING: 2>) >>> 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=<App.on_notify>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:749
Pressed() >>> MyApp(title='MyApp', classes={'-dark-mode'}) method=None
[08:23:50] WORKER                                                                                                                                                                                             worker.py:366
<Worker RUNNING name='action_pop_current_screen' description='<coroutine object MyApp.action_pop_current_screen at 0x7f99c3f2f0d0>'>
[08:23:50] SYSTEM                                                                                                                                                                                               app.py:2494
FirstScreen() is active
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
ScreenResume() >>> FirstScreen() method=<Screen.on_screen_resume>
[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=<Toast.on_mount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Mount() >>> Toast() method=<Widget.on_mount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
ScreenSuspend() >>> SecondScreen() method=<Screen.on_screen_suspend>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Blur() >>> Button() method=<Widget.on_blur>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Mount() >>> Toast() method=<Toast.on_mount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Mount() >>> Toast() method=<Widget.on_mount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Mount() >>> ToastHolder(id='--textual-toast-e34262f4-1c0b-4ec1-b2ad-79a4b57ab4b4') method=<Widget.on_mount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Unmount() >>> Tooltip(id='textual-tooltip') method=<Widget.on_unmount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Unmount() >>> Label() method=<Widget.on_unmount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Mount() >>> ToastHolder(id='--textual-toast-e34262f4-1c0b-4ec1-b2ad-79a4b57ab4b4') method=<Widget.on_mount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Unmount() >>> HeaderIcon() method=<Widget.on_unmount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Unmount() >>> HeaderTitle() method=<Widget.on_unmount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Unmount() >>> HeaderClockSpace() method=<Widget.on_unmount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Unmount() >>> FooterKey() method=<Widget.on_unmount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Unmount() >>> FooterKey() method=<Widget.on_unmount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Unmount() >>> FooterKey() method=<Widget.on_unmount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Unmount() >>> Header() method=<Widget.on_unmount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Unmount() >>> Footer() method=<Footer.on_unmount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Unmount() >>> Footer() method=<Widget.on_unmount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Unmount() >>> Button() method=<Widget.on_unmount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Unmount() >>> Toast() method=<Widget.on_unmount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Unmount() >>> ToastHolder(id='--textual-toast-e34262f4-1c0b-4ec1-b2ad-79a4b57ab4b4') method=<Widget.on_unmount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Unmount() >>> ToastRack(id='textual-toastrack') method=<Widget.on_unmount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Unmount() >>> SecondScreen() method=<Widget.on_unmount>
[08:23:50] WORKER                                                                                                                                                                                             worker.py:372
<Worker CANCELLED name='action_pop_current_screen' description='<coroutine object MyApp.action_pop_current_screen at 0x7f99c3f2f0d0>'>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Unmount() >>> Tooltip(id='textual-tooltip') method=<Widget.on_unmount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Unmount() >>> Label() method=<Widget.on_unmount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Unmount() >>> HeaderIcon() method=<Widget.on_unmount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Unmount() >>> HeaderTitle() method=<Widget.on_unmount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Unmount() >>> HeaderClockSpace() method=<Widget.on_unmount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Unmount() >>> FooterKey() method=<Widget.on_unmount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Unmount() >>> FooterKey() method=<Widget.on_unmount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Unmount() >>> FooterKey() method=<Widget.on_unmount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Unmount() >>> Header() method=<Widget.on_unmount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Unmount() >>> Footer() method=<Footer.on_unmount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Unmount() >>> Footer() method=<Widget.on_unmount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Unmount() >>> Toast() method=<Widget.on_unmount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Unmount() >>> ToastHolder(id='--textual-toast-e34262f4-1c0b-4ec1-b2ad-79a4b57ab4b4') method=<Widget.on_unmount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Unmount() >>> ToastRack(id='textual-toastrack') method=<Widget.on_unmount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Unmount() >>> FirstScreen() method=<Widget.on_unmount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Unmount() >>> ToastRack(id='textual-toastrack') method=<Widget.on_unmount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Unmount() >>> Tooltip(id='textual-tooltip') method=<Widget.on_unmount>
[08:23:50] EVENT                                                                                                                                                                                        message_pump.py:740
Unmount() >>> Screen(id='_default') method=<Widget.on_unmount>


───────────────────────────────────────────────────────────────────────────────────────────── 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
Copy link
Author

mzebrak commented Sep 26, 2024

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
Copy link
Author

mzebrak commented Sep 27, 2024

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: #5064

@matmaer
Copy link

matmaer commented Sep 27, 2024

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
Copy link
Author

mzebrak commented Sep 27, 2024

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()

Copy link

github-actions bot commented Oct 1, 2024

Don't forget to star the repository!

Follow @textualizeio for Textual updates.

Gandalf-the-Grey pushed a commit to openhive-network/clive that referenced this issue Oct 10, 2024
Gandalf-the-Grey pushed a commit to openhive-network/clive that referenced this issue Oct 10, 2024
Gandalf-the-Grey pushed a commit to openhive-network/clive that referenced this issue Oct 10, 2024
Gandalf-the-Grey pushed a commit to openhive-network/clive that referenced this issue Oct 14, 2024
Gandalf-the-Grey pushed a commit to openhive-network/clive that referenced this issue Oct 15, 2024
Gandalf-the-Grey pushed a commit to openhive-network/clive that referenced this issue Oct 15, 2024
Gandalf-the-Grey pushed a commit to openhive-network/clive that referenced this issue Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants