Deadlock/freeze when awaiting methods related to screen management in @on message handler

[matkudela]

This issue is related to #5008 (comment) - problem is still valid - awaiting pop_screen freezes application. Given workaround solution by Will is not satisfying - it can cause other issues like race conditions. I am running out of solutions for this. Of course executing the same code by bindings works without any problems. Popping screen should work not matter what caused execution of code - reaction for binding in action method or button handle in @on(Button.Pressed).
Here is MRE which shows the problem:

from __future__ import annotations

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, Static


class InitialScreen(Screen):
    def compose(self) -> ComposeResult:
        yield Static("Initial screen")
        yield Button("Push next screen")

    @on(Button.Pressed)
    def push_next_screen(self) -> None:
        self.app.push_screen(NextScreen())


class NextScreen(Screen):
    BINDINGS = [
        Binding("d", "do_something", "Do something"),
    ]

    def compose(self) -> ComposeResult:
        yield Static("Next screen")
        yield Button("Do something")
        yield Footer()

    async def action_do_something(self) -> None:
        """It's fine to call this from key bindings."""
        await self._do_something()

    @on(Button.Pressed)
    async def do_something_from_button(self) -> None:
        """Will deadlock the app."""
        await self._do_something()
        # workaround solution which can cause another issues like race conditions
        # self.app.run_worker(self._do_something())

    async def _do_something(self) -> None:
        await self.app.pop_screen()
        self.notify("Something that requires the screen to be already popped aka. await screen replacement.")


class MyApp(App):
    def on_mount(self) -> None:
        self.push_screen(InitialScreen())


MyApp().run()

A bit more complicated, but real example:

from __future__ import annotations

from textual import on
from textual.app import App, ComposeResult
from textual.binding import Binding
from textual.containers import Vertical
from textual.screen import ModalScreen, Screen
from textual.widgets import Button, Footer, Static


class ConfigScreen(Screen):
    BINDINGS = [
        Binding("escape", "app.pop_screen", "Go back"),
    ]

    def __init__(self, action: str) -> None:
        super().__init__()
        self.action = action

    def compose(self) -> ComposeResult:
        yield Static(f"Config screen of {self.action}")
        yield Footer()


class SelectConfigDialog(ModalScreen):
    DEFAULT_CSS = """
    SelectConfigDialog {
        align: center middle;
        background: $background 85%;

        Vertical {
            width: 50%;
            height: 50%;
            border: $primary outer;
            background: $panel;
        }
    }
    """

    BINDINGS = [
        Binding("escape", "dismiss", "Go back"),
        Binding("1", "push_config_screen('change-email')", "Change email"),
        Binding("2", "push_config_screen('change-password')", "Change password"),
    ]

    def compose(self) -> ComposeResult:
        with Vertical():
            yield Static("What do you want to do?")
            yield Button("Change email (1)", id="change-email")
            yield Button("Change password (2)", id="change-password")
        yield Footer()

    @on(Button.Pressed)
    async def push_config_screen_from_button(self, event: Button.Pressed) -> None:
        """Should dismiss the dialog and push the appropriate config screen."""
        # await self.dismiss()  # Will cause ScreenError("Can't await screen.dismiss() from the screen's message handler; try removing the await keyword.")
        # self.dismiss()  # AFAIK calling without await is dangerous as it may dismiss ConfigScreen instead of SelectConfigDialog by looking at the `await_pop = self.app.pop_screen()` in dismiss method
        await self.app.pop_screen()  # Will cause the deadlock
        self.app.push_screen(ConfigScreen(event.button.id))

    async def _action_push_config_screen(self, action: str) -> None:
        """Should dismiss the dialog and push the appropriate config screen."""
        await self.dismiss()  # works just fine
        self.app.push_screen(ConfigScreen(action))


class InitialScreen(Screen):
    BINDINGS = [
        Binding("c", "open_configuration_dialog", "Open configuration dialog"),
    ]

    def compose(self) -> ComposeResult:
        yield Static("Initial screen")
        yield Static("Press 'c' to open the dialog.")
        yield Footer()

    def action_open_configuration_dialog(self) -> None:
        self.app.push_screen(SelectConfigDialog())


class MyApp(App):
    def on_mount(self) -> None:
        self.push_screen(InitialScreen())


MyApp().run()

In my application also faced similar problem while switching modes. I have dashboard (main) mode and config mode. Also got some global buttons in header which change modes if necessary - dashboard, config and cart (which is in dashboard mode, additionally pushes next screen). This time application freezes probably while resetting old mode (need to remove and add it to drop all screens from screen stack). I think this is connected with pop_screen problem. Both are screen management.

from __future__ import annotations

from typing import Literal

from textual import on
from textual.app import App, ComposeResult, ScreenError
from textual.await_complete import AwaitComplete
from textual.binding import Binding
from textual.containers import Horizontal
from textual.screen import Screen, ScreenResultType
from textual.widgets import Button, Footer, Static


class GlobalButtons(Horizontal):
    """In real scenario, this can be e.g in the header or hamburger menu.."""

    DEFAULT_CSS = """
    GlobalButtons {
        align: right bottom;
        margin-bottom: 1;
        height: auto;
    }
    """
    def compose(self) -> ComposeResult:
        yield Button("Dashboard", id="main")
        yield Button("Config", id="config")
        yield Button("Cart", id="cart")
        yield Button("Account", id="account")

    @on(Button.Pressed)
    async def handle_navigation(self, event: Button.Pressed) -> None:
        #await self.app.handle_navigation(event.button.id) # Will cause deadlock
        self.app.run_worker(self.app.handle_navigation(event.button.id))   # This workaround can cause race when pressing buttons too fast e.g. ActiveModeError: Can't remove active mode 'main'


class CartScreen(Screen):
    """Cart should be pushed only in the main mode."""

    BINDINGS = [
        Binding("escape", "app.pop_screen", "Go back"),
    ]

    def compose(self) -> ComposeResult:
        yield GlobalButtons()
        yield Static("Cart")
        yield Footer()

class AccountScreen(Screen):
    """Account should be pushed only in the main mode."""

    BINDINGS = [
        Binding("escape", "app.pop_screen", "Go back"),
    ]

    def compose(self) -> ComposeResult:
        yield GlobalButtons()
        yield Static("Account")
        yield Footer()


class DashboardScreen(Screen):
    def compose(self) -> ComposeResult:
        yield GlobalButtons()
        yield Static("Dashboard")
        yield Footer()


class ConfigScreen(Screen):
    def compose(self) -> ComposeResult:
        yield GlobalButtons()
        yield Static("Config screen")
        yield Footer()


class MyApp(App):
    BINDINGS = [
        Binding("d", "handle_navigation('main')", "Dashboard"),
        Binding("c", "handle_navigation('config')", "Config"),
        Binding("t", "handle_navigation('cart')", "Cart"),
        Binding("u", "handle_navigation('account')", "Account"),
    ]
    """Global bindings works just fine with no deadlock."""

    MODES = {
        "config": ConfigScreen,
        "main": DashboardScreen,
    }

    def on_mount(self) -> None:
        self.switch_mode("main")

    async def action_handle_navigation(self, requested: Literal["cart", "account", "config", "main"]) -> None:
        await self.handle_navigation(requested)

    async def handle_navigation(self, requested: Literal["cart", "account", "config", "main"]) -> None:
        main_push_transitions = {
            ("main", "cart"): CartScreen,
            ("main", "account"): AccountScreen,
        }

        config_switch_to_main_and_push_transitions = {
            ("config", "cart"): CartScreen,
            ("config", "account"): AccountScreen,
        }

        if isinstance(self.screen, CartScreen) and requested == "cart" or isinstance(self.screen, AccountScreen) and requested == "account":
            # nothing to do
            return

        if self.current_mode == "main" and requested == "main":
            self.get_screen_from_current_stack(DashboardScreen).pop_until_active()
            return

        if (key := (self.current_mode, requested)) in main_push_transitions:
            await self.push_screen(main_push_transitions[key]())
            return

        if (key := (self.current_mode, requested)) in config_switch_to_main_and_push_transitions:
            # Transitioning from config mode to main mode before pushing the screen
            with self.app.batch_update():
                await self.switch_mode_with_reset("main")
                await self.push_screen(config_switch_to_main_and_push_transitions[key]())
            return

        if requested in {"main", "config"}:
            await self.switch_mode_with_reset(requested)
            return

        raise AssertionError(f"Unexpected navigation request: {requested}")

    def switch_mode_with_reset(self, new_mode: str) -> AwaitComplete:
        """
        Switch mode and reset all other active modes.

        The `App.switch_mode` method from Textual keeps the previous mode in the stack.
        This method allows to switch to a new mode and have only a new mode in the stack without keeping
        any screen stacks of other modes. The next switch mode will be like fresh switch mode.

        Args:
        ----
            new_mode: The mode to switch to.
        """

        async def impl() -> None:
            await self.switch_mode(new_mode)

            modes_to_keep = (new_mode, "_default")
            modes_to_reset = [mode for mode in self._screen_stacks if mode not in modes_to_keep]
            assert all(mode in self.MODES for mode in modes_to_reset), "Unexpected mode in modes_to_reset"
            await self.reset_mode(*modes_to_reset)

        return AwaitComplete(impl()).call_next(self)

    def reset_mode(self, *modes: str) -> AwaitComplete:
        async def impl() -> None:
            for mode in modes:
                await self.remove_mode(mode)
                self.add_mode(mode, self.MODES[mode])

        return AwaitComplete(impl()).call_next(self)

    def get_screen_from_current_stack(self, screen: type[Screen[ScreenResultType]]) -> Screen[ScreenResultType]:
        for current_screen in self.screen_stack:
            if isinstance(current_screen, screen):
                return current_screen
        raise ScreenError(f"Screen {screen} not found in stack")

MyApp().run()

[github-actions]

We found the following entries in the FAQ which you may find helpful:

• Does Textual support images?
• How do I center a widget in a screen?
• How do I pass arguments to an app?

Feel free to close this issue if you found an answer in the FAQ. Otherwise, please give us a little time to review.

This is an automated reply, generated by FAQtory

[willmcgugan]

Your first and second examples create a deadlock by awaiting pop_screen in a message handlers. The screen can't be popped until it has processed all messages. This can be fixed by not awaiting pop_screen. But ideally you should be using dismiss which is the recommend way for screens to pop themselves.

Your third example is doing some things with regards to modes which seems odd to me, and I am reluctant to comment on until I understand your use case. If I understood what problem you think you are solving, I suspect I could offer an alternative solution which would negate the need for "reseting modes".

To understand your use case fully, I will need to know why you are doing it that way. Please describe that problem for me, rather than the solution you are implementing.

[matkudela]

Your first and second examples create a deadlock by awaiting pop_screen in a message handlers. The screen can't be popped until it has processed all messages.

Yeah, that's exactly why I created this issue. This is the root of the problem. Awaiting in a message handler creates a deadlock, while awaiting in a binding handler works just fine - I think it should work just fine in both cases.

Currently, it leads to a hard-to-notice bug, which is a major one (because it's a deadlock) and prevents doing some things via button widget while it’s just fine to do it via key binding.

Another example with new Button action parameter

from __future__ import annotations

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, Static


class FirstScreen(Screen):
    def compose(self) -> ComposeResult:
        yield Static("First screen")
        yield Button("Push second screen")

    @on(Button.Pressed)
    def push_second_screen(self) -> None:
        self.app.push_screen(SecondScreen())


class SecondScreen(Screen):
    BINDINGS = [
        Binding("d", "do_something", "Do something"),
    ]

    def compose(self) -> ComposeResult:
        yield Static("Second screen")
        yield Button("Do something", action="do_something")
        yield Footer()

    async def action_do_something(self) -> None:
        """It's fine to call this from key bindings."""
        await self.app.pop_screen()
        self.notify("Something that requires the screen to be already popped aka. await screen replacement.")


class MyApp(App):
    def on_mount(self) -> None:
        self.push_screen(FirstScreen())


MyApp().run()

This is also the same issue (which was workarounded):

• Using dismiss in call_after_refresh now raises an error. #5049

This might also be the root issues of these:

• pop_until_active does not pop screens #5166 (looks like it's very related)
• Footer binding disappear #5153 (comment) (It looks like dismiss has to happen there so I think it’s also related)
• Fix sporadic NoMatches error reported from Header on app shutdown #4817 / NoMatches error occurring when changing screens quickly in version 0.49+ #4268 (reply in thread) (also related to dismiss / pop_screen)

This can be fixed by not awaiting pop_screen.

What do you mean by not awaiting pop_screen while as I said, I need to be sure, that screen was changed before proceeding with further instructions.

When we have to await it and make sure that the previous screen was popped, this just can’t

be fixed by not awaiting pop_screen

AFAIK, this can be achieved only by awaiting the dismiss/pop_screen.

I also know the workaround about awaiting in a different task (run_worker/work) like mentioned there and there, and yeah, this workaround solves the issue with deadlock but introduces another one - race condition, because we’re running code concurrently in a separate tasks.

I don’t think there’s a proper fix/way to do it but rather a workaround that introduces another issue.

To understand your use case fully, I will need to know why you are doing it that way. Please describe that problem for me, rather than the solution you are implementing.

The Textual mode switching via App.switch_mode keeps screen stacks from multiple modes in “history” as it can be observed in App._screen_stacks.I want them to be discarded instead.

So, let’s assume I’m in main mode abd in my screen_stacks I have e.g:
main: Dashboard, SomeScreen, SomeOtherScreen

and I switch mode main -> config like switch_mode(“config”), then “main” screen stack will remain and I’ll have:
main: Dashboard, Products, Cart
config: Config

I don’t want that behaviour. I want the previous “mode” to be discarded so when I switch main -> config like switch_mode(“config”), there will be just the config:
- config: Config

So then, when I come back, config -> main like switch_mode(“main”) there will be again just the freshly initialized dashboard:
main: Dashboard

I think it is a matter of choice. I think some apps will want to keep the previous mode intact when changing mode to another one, so they can go back to the same place when switching mode again, but others will want to go back to the initial screen from the mode. In my project, I need to switch always to the "fresh" mode - so initial screen of it.

Another problem is that leaving many modes/screens existing leads to their refreshing (reactive) even though they are not the currently displayed screen/mode.

So that's why I have implemented switch_mode_with_reset.

[willmcgugan]

Your first and second examples create a deadlock by awaiting pop_screen in a message handlers. The screen can't be popped until it has processed all messages.

Yeah, that's exactly why I created this issue. This is the root of the problem. Awaiting in a message handler creates a deadlock, while awaiting in a binding handler works just fine - I think it should work just fine in both cases.

It is a gotcha, but it is not a bug in Textual. You can create such deadlocks in asyncio without textual.

Awaiting pop_screen means waiting until all messages in a screen have been processed. Logically this cannot happen if your message handler is blocking. There is no way to make it "work just fine" without breaking what it means to await the screen. Textual is not magic. The best Textual can do is raise a helpful error when you attempt this, which is what dismiss does.

The solution is to not await pop_screen or dismiss. That's why awaiting is optional. You can also run your handler concurrently with with a worker. If you have race conditions, this can be solved through synchronization primitives which you will find in the asyncio documentation.

The solution(s) I described are canonical, and frankly perfectly fine. If you still don't like it, I'm afraid I can't help you any further.

With regards to your switching mode issue. I'm almost certain I don't follow fully, but perhaps I am getting closer. I will close this issue, and I suggest you open a discussion (not an issue). Please explain what you are attempting to solve from the user's perspective. In other words, avoid mentioning modes, or screen stacks etc. Focus on the user's experience.

[github-actions]

Don't forget to star the repository!

Follow @textualizeio for Textual updates.