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.19k stars 772 forks source link

Deadlock when closing app, associated with Footer.recompose and focus events #4677

Closed slawek-es closed 3 months ago

slawek-es commented 3 months ago

A deadlock similar to the one described in #4643 occurs in a simple test case where the application is closed very soon after it gets initialized. The supposed fix for #4643 does not work for this case, and the deadlock is still reproducible with textual version 0.70.0.

The issue seems to again be a bad interaction between the Footer.recompose call run as the response to the bindings_updated signal and the procedure of closing the entire application. The Footer.recompose enqueues an AwaitRemove callback while the _dom_lock is already held by _close_all, and closing the Footer requires completing the AwaitRemove future, leading to a deadlock. The issue seems to be triggered by set_focus calls which we do from a background task.

Minimal Reproduction Environment Run `pytest editshare_system_upgrade/textual_ui/repro_app.py` to see the `TimeoutError`: ``` import asyncio from textual import work from textual.app import App, ComposeResult from textual.widgets import Header, Footer, Input from textual.screen import Screen class ReproScreen(Screen[None]): def __init__(self) -> None: super().__init__() self._input = Input() def on_mount(self) -> None: """Start a background refresh loop.""" self.refresh_state() def compose(self) -> ComposeResult: yield Header() yield self._input yield Footer() @work async def refresh_state(self) -> None: """This stands for a more complicated refresh state loop.""" while True: self.set_focus(self._input) await asyncio.sleep(0.1) class ReproApp(App[None]): AUTO_FOCUS = None # Important for reproduction! def __init__(self) -> None: super().__init__() async def on_mount(self) -> None: self.initialize_welcome_screen() @work async def initialize_welcome_screen(self) -> None: await self.push_screen(ReproScreen()) async def test_repro_app() -> None: app = ReproApp() async with app.run_test(): pass if __name__ == "__main__": asyncio.run(ReproApp().run_async()) ```
Output from `textual diagnose` ``` # Textual Diagnostics ## Versions | Name | Value | |---------|--------| | Textual | 0.70.0 | | Rich | 13.7.1 | ## Python | Name | Value | |----------------|--------------------------------------------------------------| | Version | 3.12.1 | | Implementation | CPython | | Compiler | GCC 9.4.0 | | Executable | /home/saf/code/es/editshare-system-upgrade/.venv/bin/python3 | ## Operating System | Name | Value | |---------|----------------------------------------------| | System | Linux | | Release | 5.4.0-186-generic | | Version | #206-Ubuntu SMP Fri Apr 26 12:31:10 UTC 2024 | ## Terminal | Name | Value | |----------------------|-----------------| | Terminal Application | vscode (1.90.2) | | TERM | xterm | | COLORTERM | truecolor | | FORCE_COLOR | *Not set* | | NO_COLOR | *Not set* | ## Rich Console options | Name | Value | |----------------|----------------------| | size | width=169, height=18 | | legacy_windows | False | | min_width | 1 | | max_width | 169 | | is_terminal | False | | encoding | utf-8 | | max_height | 18 | | justify | None | | overflow | None | | no_wrap | False | | highlight | None | | markup | None | | height | None | ```

Feel free to add screenshots and / or videos. These can be very helpful!

github-actions[bot] commented 3 months ago

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

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 commented 3 months ago

Have you managed to reproduce this outside of pytest?

slawek-es commented 3 months ago

Have you managed to reproduce this outside of pytest?

Well, if you change the reproduction module to say:

if __name__ == "__main__":
    asyncio.run(test_repro_app())

then the issue technically happens outside of pytest, but it still uses App.run_test. Do you think this is a deciding factor here?

Apart from this, I have been unable to reproduce this issue by running the application normally.

willmcgugan commented 3 months ago

Fixed in main.

github-actions[bot] commented 3 months ago

Don't forget to star the repository!

Follow @textualizeio for Textual updates.