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

Trio warning when exiting the application #262

Open
vxgmichel opened this issue Jul 22, 2021 · 13 comments
Open

Trio warning when exiting the application #262

vxgmichel opened this issue Jul 22, 2021 · 13 comments

Comments

@vxgmichel
Copy link

vxgmichel commented Jul 22, 2021

Hi @altendky,

When exiting our qtrio application (which runs with qtrio.run), we sometimes get the following warning:

.../python3.8/site-packages/trio/_core/_run.py:2221: RuntimeWarning: Trio guest run got abandoned without properly finishing... weird stuff might happen

The comment here says something about garbage collection:

https://github.com/python-trio/trio/blob/3f4e1a23e567825926b225f87c64db56d95f96fb/trio/_core/_run.py#L2247-L2253

Do you have some ideas about what could cause this?

@vxgmichel
Copy link
Author

vxgmichel commented Jul 22, 2021

Also, I tried to reproduce the issue with a minimal example but I couldn't. There's probably more to it, here's my attempt:

import trio
import qtrio

from PyQt5.QtWidgets import QApplication, QMainWindow


async def task():
    stream = await trio.open_tcp_stream("localhost", 8000)
    await stream.send_all(b"test")
    await stream.receive_some()


async def main():
    window = QMainWindow()
    window.show()
    async with trio.open_nursery() as nursery:
        nursery.start_soon(task)
        await trio.sleep(1)
        window.close()
        QApplication.quit()

if __name__ == "__main__":
    qtrio.run(main)

Run with:

nc localhost -l 8000 & python test.py

@vxgmichel
Copy link
Author

I realized QApplication.quit() was the issue as it causes the qt loop to terminate without letting the trio loop terminate as well. However, the warning is not generated in all cases. Consider the following example that silently fails (it doesn't print Very important stuff!) without any trio warning:

import trio
import qtrio

from PyQt5.QtWidgets import QApplication, QMainWindow


async def task(window):
    await trio.sleep(1)
    window.close()
    QApplication.quit()



async def main():
    try:
        window = QMainWindow()
        window.show()
        async with trio.open_nursery() as nursery:
            nursery.start_soon(task, window)
            await trio.sleep(3)
    finally:
        print("Very important stuff!")

if __name__ == "__main__":
    qtrio.run(main)

@altendky
Copy link
Owner

altendky commented Jul 22, 2021

Do you need to call QApplication.quit()? Can you just return from your main() and let QTrio handle the application?

self.application.quit()

https://qtrio.readthedocs.io/en/v0.4.2/core.html#qtrio.Runner.quit_application

quit_application
    When true, the done_callback() method will quit the application
    when the async function passed to qtrio.Runner.run() has completed.

@vxgmichel
Copy link
Author

Do you need to call QApplication.quit() ?

I don't, it's simply an issue I ran into while migrating our code base. I think the problem I described in my last comment is quite serious as it's a common pattern to use methods like loop.stop() in asyncio or app.quit() in qt to stop the execution of an event loop, and silently missing finally clauses is a big deal.

I just ran into this section in the trio documentation:

You must let the Trio program run to completion. Many event loops let you stop the event loop at any point, and any pending callbacks/tasks/etc. just… don’t run. Trio follows a more structured system, where you can cancel things, but the code always runs to completion, so finally blocks run, resources are cleaned up, etc. If you stop your host loop early, before the done_callback is invoked, then that cuts off the Trio run in the middle without a chance to clean up. This can leave your code in an inconsistent state, and will definitely leave Trio’s internals in an inconsistent state, which will cause errors if you try to use Trio again in that thread.

Some programs need to be able to quit at any time, for example in response to a GUI window being closed or a user selecting a “Quit” from a menu. In these cases, we recommend wrapping your whole program in a trio.CancelScope, and cancelling it when you want to quit.

Maybe qtrio could wrap the execution of async_fn with a cancel scope and expose a method in the qtrio runner to stop the application properly? Also it'd be nice if qtrio could detect those incorrect shutdown of the event loop and show a warning advising against the use of QApplication.quit(). I think a warning in the documentation would also be useful.

I hope this makes sense :)

@altendky
Copy link
Owner

There's qtrio.Runner.cancel_scope.

  • cancel_scope
    • An all encompassing cancellation scope for the Trio execution.

I'll have to look more at the other bits to refresh myself. We might be able to check if the application is running at the end of every Trio callback into the Qt loop.

@altendky
Copy link
Owner

So looking back at this, it appears that there is already a RuntimeWarning that gets raise. It just references directly to Trio rather than QTrio. Would that warning customized for QTrio be something that would fulfill your interests? (plus discussion in the documentation, I haven't looked there yet)

@vxgmichel
Copy link
Author

vxgmichel commented Jul 26, 2021

So looking back at this, it appears that there is already a RuntimeWarning that gets raise.

The warning is not always raised. Consider the example below:

import trio
import qtrio

from PyQt5.QtWidgets import QApplication, QMainWindow


async def task(window):
    await trio.sleep(1)
    window.close()
    QApplication.quit()



async def main():
    try:
        window = QMainWindow()
        window.show()
        async with trio.open_nursery() as nursery:
            nursery.start_soon(task, window)
            await trio.sleep(3)
    finally:
        print("Very important stuff!")

if __name__ == "__main__":
    qtrio.run(main)

See how Very important stuff is never printed and no warning gets displayed.

@richardsheridan
Copy link

Is it possible to monkeypatch QApplication.quit? That might be a way for qtrio to put a relevant warning up, if there's not really a legitimate use for it except in done_callback which qtrio handles.

@graingert
Copy link

Is it possible to monkeypatch QApplication.quit? That might be a way for qtrio to put a relevant warning up, if there's not really a legitimate use for it except in done_callback which qtrio handles.

apparently you can listen to aboutToQuit which runs after user input is disabled and before the event loop is closed

@njsmith
Copy link

njsmith commented Jul 26, 2021

Can you call exec repeatedly on the same QApplication? If so then I guess qtrio could detect when exec exits before trio, and just reinvoke exec, effectively making QApplication.quit a no-op.

The aboutToQuit signal does seem like it could at least reliably detect this case and give a good warning.

@altendky
Copy link
Owner

I'm not looking for a bunch of work right now so I wasn't going to take on actively keeping the loop going. Perhaps the monkey patching to disable quitting would be good, but I'm trying to muck around less, not more. Maybe later. For now I put in a warning triggered by aboutToQuit() (#267). I'll take a look over it later. At least the docs could be fleshed out with some more things that can cause this. Perhaps they can be better integrated with the rest of the documentation as well.

@altendky
Copy link
Owner

Without agreeing to take it on now, it sounds like actually making sure everything runs in .aboutToQuit() will be needed.

https://doc.qt.io/qt-5/qcoreapplication.html#exec

We recommend that you connect clean-up code to the aboutToQuit() signal, instead of putting it in your application's main() function because on some platforms the exec() call may not return. For example, on Windows when the user logs off, the system terminates the process after Qt closes all top-level windows. Hence, there is no guarantee that the application will have time to exit its event loop and execute code at the end of the main() function after the exec() call.

thanks for that...

@njsmith
Copy link

njsmith commented Jul 26, 2021

while you could in theory block in aboutToQuit and run the trio guest loop to completion, I'm not sure it's worth trying... the problem is that the trio code is probably written to assume that the qt loop is available, so resuming it in an environment where it can't use qt apis isn't necessarily any better than just quitting with a loud warning.

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

No branches or pull requests

5 participants