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

What is the intended way of blocking synchronous slots until some async. code is done? #33

Open
rutsky opened this issue Jun 25, 2015 · 12 comments
Assignees
Milestone

Comments

@rutsky
Copy link

rutsky commented Jun 25, 2015

Sometimes it is required to wait for result of asynchronous operation in synchronous Qt slot, is there any proper way to do it?

For example I have QMainWindow subclass that has QWebEngineView as one of it's child, QWebEngineView runs some JavaScript client code, and my application communicates with it asynchronously using WebSockets.
Now I want to request from QWebEngineView some data to store on application exit: in overridden QMainWindow::closeEvent().

I can't create async. task and let closeEvent() to finish, since the main window will destroy QWebEngineView on close, and most probably I won't get results in time.
I can't use BaseEventLoop.run_until_complete() with my async. call, since event loop is already running and is not reentrant.
I can't create second thread, start second event loop there, call async. method in the second event loop, and in closeEvent() wait when the thread will finish, since my web socket is already managed by event loop in the main thread.

My current solution is to postpone the main window closing by ignoring first close event, start async. task, and call QMainWindow::close() second time manually from async. task when work is done.
This solution works in closeEvent(), since it's possible to postpone closing, but will not in most of other synchronous slots.

@harvimt
Copy link
Owner

harvimt commented Jun 25, 2015

create a future, pass the future to the event handler, pause on the future

future = asyncio.Future()
yield from future #waits until future.set_result() has been called

I can't use BaseEventLoop.run_until_complete() with my async. call, since event loop is already running and is not reentrant.

This is actually probably safe, but only because of the Qt backend, in general, run_until_complete isn't reentrant.

It seems like you could just have the web view store the data outside itself so it doesn't get destroyed when itself is destroyed.

@rutsky
Copy link
Author

rutsky commented Jun 25, 2015

create a future, pass the future to the event handler, pause on the future

Not sure that I'm properly understand you. I need to run async. code from synchronous function:

@asyncio.coroutine
def f():
    ...
    yield from ...
    ...
    return result

class MainWindow(QMainWindow):
    ...
    def closeEvent(self, event):
        # I need to call f() here and wait till it finishes
        # I can't use:
        #result = yield from f()
        # because closeEvent() is not coroutine.
        # I can't schedule execution of f(), because I need to wait for result before returning from closeEvent()
        #loop.create_task(f())
        ...

This is actually probably safe, but only because of the Qt backend, in general, run_until_complete isn't reentrant.

Looks like Quamash doesn't support reentrancy in event loop: I tried to use run_until_complete() in closeEvent() and it failed with exception, if I correctly remember, that (QEventLoop)exec() can not be called, because it's already inside exec().

Probably this can be fixed? If yes, this will solve the issue with waiting for async. code in sync. Qt functions.

It seems like you could just have the web view store the data outside itself so it doesn't get destroyed when itself is destroyed.

I query from web view information about current state (web view displays map of the world and I want to preserve position on the map between application restarts), so it cannot be moved outside web view. According to QWebEngineView documentation, QWebEngineView owns web engine and it's lifetime is the same, as the lifetime of QWebEngineView widget.

@harvimt
Copy link
Owner

harvimt commented Jun 26, 2015

Sorry for the misunderstanding, you want to do precisely the opposite of what I thought.
Someone else wanted to do this and the recommendation was to wrap everything in asyncio.async but I gather that's not an option because you need to delay returning from closeEvent or else resources will be released that you need.

Hrm. This should maybe be easier. (quamash is pre 1.0 software after all)

Can you use QtCore.QEventLoop?

class MainWindow(QMainWindow):
    ...
    def closeEvent(self, event):
        qloop = QtCore.QEventLoop()
        loop = quamash.QEventLoop(qloop)
        result = loop.run_until_complete(f())

even more hackish

class MainWindow(QMainWindow):
    ...
    def closeEvent(self, event):
        fut = asyncio.async(f())
        while not fut.done():
             app.processEvents()
             time.sleep(0.1)
        result = fut.result()

@harvimt harvimt self-assigned this Jun 26, 2015
@horazont
Copy link
Contributor

I think you still can do what was proposed to me in #11. The reason is that the close will not proceed until you call super().closeEvent(), and you can just do that from within the asynchronous function.

# get asyncify from #11 
@asyncify
@asyncio.coroutine
def closeEvent(self, ev):
    yield from some_long_task()
    super().closeEvent(ev)

From what I suspect, Qt implements the actual closing in the inherited closeEvent. If it does not work, you can still check out my proposed implementation of inline_async. That should be legal (QDialog::exec is doing it, too), but not as nice as asyncify.

(In fact, it not only works for me with closeEvent, but also with most other synchronous slots and overriden methods. I have not seen any negative side effect. The reason seems to be that most interesting work is in fact done in the inherited methods. And you can just decide to call them from within the asynchronous task at a later point in time)

@rutsky
Copy link
Author

rutsky commented Jul 3, 2015

FYI, if I use

...
def closeEvent(self, event):
    loop.run_until_complete(f())

I get following exception:

QCoreApplication::exec: The event loop is already running
Traceback (most recent call last):
  File "/home/bob/main_window.py", line 112, in closeEvent
    loop.run_until_complete(f())
  File "/home/bob/env/lib/python3.4/site-packages/quamash/__init__.py", line 290, in run_until_complete
    raise RuntimeError('Event loop stopped before Future completed.')
RuntimeError: Event loop stopped before Future completed.

@harvimt,

Can you use QtCore.QEventLoop?

Running second event loop actually works in my configuration exactly as you suggested:

class MainWindow(QMainWindow):
    ...
    def closeEvent(self, event):
        qloop = QtCore.QEventLoop()
        loop = quamash.QEventLoop(qloop)
        result = loop.run_until_complete(f())

But I don't think it's guaranteed behaviour: I/O primitive should be registered in an event loop, and I think it's not safe to use single I/O primitive in multiple event loops.
In my configuration the event loop uses select.select-based I/O demultiplexing, which allows using single I/O primitive in mutliple select.select calls, but I think this is not true for other selectors or when asyncio.ProactorEventLoop is used.

@horazont, you are right that only calling super().closeEvent() will cause window to close, but allowing return from my overridden closeEvent() leads to increase of program logic: I need to handle case when closeEvent() is called multiple times, and I'm not sure that I can store event, because Qt should destroy (or reuse) it while event processing finished (when closeEvent() handler returned).

In general I need what you, @horazont , suggested in PR #11 : inline_async, and it's implementation is practically that you, @harvimt , suggested, but as I stated before I think such implementation is not guaranteed to work on any low-level event loop implementation.

@rutsky
Copy link
Author

rutsky commented Jul 3, 2015

It would be great if Quamash will guarantee, that running second event loop is safe (e.g. by using on low level event loop implementation that allows sharing of I/O primitives between event loops).
Then it would be legal to use @horazont inline_async wrapper or suggested by @harvimt manual event loop managing to solve this issue.

@harvimt
Copy link
Owner

harvimt commented Jul 3, 2015

QtCore.QEventLoop isn't a second event loop, it's a second "view" of the
same event loop. check the code for PR #11, it uses QtCore.QEventLoop to do
it's thing.

On Fri, Jul 3, 2015 at 6:03 AM, Vladimir Rutsky notifications@github.com
wrote:

It would be great if Quamash will guarantee, that running second event
loop is safe (e.g. by using on low level event loop implementation that
allows sharing of I/O primitives between event loops).
Then it would be legal to use @horazont https://github.com/horazont
inline_async wrapper or suggested by @harvimt https://github.com/harvimt
manual event loop managing to solve this issue.


Reply to this email directly or view it on GitHub
#33 (comment).

@rutsky
Copy link
Author

rutsky commented Jul 3, 2015

QtCore.QEventLoop isn't a second event loop, it's a second "view" of the same event loop. check the code for PR #11, it uses QtCore.QEventLoop to do it's thing.

Yes, I believe Qt allows to create several nested QEventLoops one inside another, and Qt events will be properly dispatched.
But in example above we not only create new QEventLoop, but also create new quamash.QEventLoop which inherits asyncio.SelectorEventLoop (on Linux) or asyncio.ProactorEventLoop (on Windows).
Does multiple asyncio.SelectorEventLoop or asyncio.ProactorEventLoop instances properly handles single I/O primitive (e.g. socket, opened in "first" event loop)?

@harvimt
Copy link
Owner

harvimt commented Jul 3, 2015

yeah. that could be a problem, the obvious solution is to have each run of
run_until_complete() wrap a QtCore.QEventLoop, that might be wise, or a
seperate inline_async function, but I think modifying run_until_complete
to be reentrant might be better?

On Fri, Jul 3, 2015 at 10:47 AM, Vladimir Rutsky notifications@github.com
wrote:

QtCore.QEventLoop isn't a second event loop, it's a second "view" of the
same event loop. check the code for PR #11
#11, it uses QtCore.QEventLoop
to do it's thing.

Yes, I believe Qt allows to create several nested QEventLoops one inside
another, and Qt events will be properly dispatched.
But in example above we not only create new QEventLoop, but also create
new quamash.QEventLoop which inherits asyncio.SelectorEventLoop (on
Linux) or asyncio.ProactorEventLoop (on Windows).
Does multiple asyncio.SelectorEventLoop or asyncio.ProactorEventLoop
instances properly handles single I/O primitive (e.g. socket, opened in
"first" event loop)?


Reply to this email directly or view it on GitHub
#33 (comment).

@rutsky
Copy link
Author

rutsky commented Jul 3, 2015

yeah. that could be a problem, the obvious solution is to have each run of run_until_complete() wrap a QtCore.QEventLoop, that might be wise, or a seperate inline_async function, but I think modifying run_until_complete to be reentrant might be better?

Yes, looks like starting new QtCore.QEventLoop in each run_until_complete() should work.

Making run_until_complete to be reentrant would be convenient for end user, but this behaviour will conflict with default event loop behavior: at least default asyncio event loop on Linux is not reentrant.
There was a bug report regarding nested event loops: https://bugs.python.org/issue22239 and Guido van Rossum decided to forbid nested event loops:

While I understand your problem, I really do not want to enable recursive event loops. While they are popular in some event systems (IIRC libevent relies heavily on the concept), I have heard some strong objections from other parts, and I am trying to keep the basic event loop functionality limited to encourage interoperability with other even loop systems (e.g. Tornado, Twisted).

In my own experience, the very programming technique that you are proposing has caused some very hard to debug problems that appeared as very infrequent and hard to predict stack overflows.

I understand this will make your code slightly less elegant in some cases, but I think in the end it is for the best if you are required to define an explicit method (declared to be a coroutine) for membership testing of a remote object. The explicit "yield from" will help the readers of your code understand that global state may change (due to other callbacks running while you are blocked), and potentially help a static analyzer find bugs in your code before they take down your production systems.

Probably it would be better to explicitly use separate function for such behavior, like @horazont 's inline_async().

@gmarull
Copy link

gmarull commented Sep 21, 2018

Maybe more related to #11, but I've found this syntax (asyncSlot()) quite useful:

import sys
import asyncio
import functools
import random

from PyQt5.QtCore import pyqtSlot
from PyQt5.QtWidgets import QApplication, QWidget, QPushButton, QHBoxLayout
from quamash import QEventLoop


app = QApplication(sys.argv)
loop = QEventLoop(app)
asyncio.set_event_loop(loop)


def asyncSlot(*args):
    def real_decorator(fn):
        @pyqtSlot(*args)
        @functools.wraps(fn)
        def wrapper(*args, **kwargs):
            asyncio.ensure_future(fn(*args, **kwargs))
        return wrapper
    return real_decorator


class Window(QWidget):
    def __init__(self):
        super().__init__()
    
        self.setLayout(QHBoxLayout())

        self.btn1 = QPushButton('Ready', parent=self)
        self.btn1.clicked.connect(self.on_btn1_clicked)
        self.layout().addWidget(self.btn1)

        self.btn2 = QPushButton('Test', parent=self)
        self.layout().addWidget(self.btn2)

    @asyncSlot()
    async def on_btn1_clicked(self):
        try:
            self.btn1.setEnabled(False)
            self.btn1.setText('Running...')
            await self.task()
            self.btn1.setText('Done!')
        except Exception:
            self.btn1.setText('Error!')
        finally:
            self.btn1.setEnabled(True)

    async def task(self):
        await asyncio.sleep(1)
        if bool(random.getrandbits(1)):
            raise Exception('Oooops a random exception!')



w = Window()
w.show()


with loop:
    loop.run_forever()

@danieljfarrell
Copy link

danieljfarrell commented Sep 5, 2019

Cancelling the task?

@gmarull I really like this solution! Thanks.

Can you think of a way of cancelling the task created by asyncio.ensure_future?

The only way I could think of doing it is below.

When the cancel button is pressed I search all running tasks and match the task's coroutine's __qualname__ attribute to that of async def method name on the object. It's pretty gross.

(In asyncSlot I also added a done callback on the task so that any exceptions can be caught).

import sys
import asyncio
import functools
import random
import inspect
from PyQt5.QtCore import pyqtSlot, pyqtRemoveInputHook
from PyQt5.QtWidgets import QApplication, QWidget, QPushButton, QHBoxLayout
from quamash import QEventLoop


app = QApplication(sys.argv)
loop = QEventLoop(app)
asyncio.set_event_loop(loop)


def debug_trace():
    """Set a tracepoint in the Python debugger that works with Qt"""
    pyqtRemoveInputHook()
    import pdb; pdb.set_trace()


def asyncSlot(*args):

    def log_error(future):
        try:
            future.result()
        except asyncio.CancelledError:
            pass
        except Exception:
            # Add better error handling
            traceback.print_exc()

    def helper(coro):
        if not inspect.iscoroutinefunction(coro):
            raise RuntimeError('Must be a coroutine!')

        @pyqtSlot(*args)
        @functools.wraps(coro)
        def wrapper(self, *args, **kwargs):
            loop = asyncio.get_event_loop()
            future = loop.create_task(coro(self, *args, **kwargs))
            future.add_done_callback(log_error)
        return wrapper

    return helper


class Window(QWidget):
    def __init__(self):
        super().__init__()
    
        self.setLayout(QHBoxLayout())

        self.btn1 = QPushButton('Ready', parent=self)
        self.btn1.clicked.connect(self.onRun)
        self.layout().addWidget(self.btn1)

        self.btn2 = QPushButton('Cancel', parent=self)
        self.btn2.clicked.connect(self.onCancel)
        self.layout().addWidget(self.btn2)


    @asyncSlot()
    async def onRun(self):
        try:
            self.btn1.setEnabled(False)
            self.btn1.setText('Running...')
            await self.task()
            self.btn1.setText('Done!')
        except asyncio.CancelledError:
            self.btn1.setText('Cancelled!')
        except Exception:
            self.btn1.setText('Error!')
        finally:
            self.btn1.setEnabled(True)

    @pyqtSlot()
    def onCancel(self):
        print("Cancelling.")
        for t in asyncio.Task.all_tasks():
            if t._coro.__qualname__ == self.onRun.__qualname__:
                t.cancel()
                print("Cancelled.")
                break

    async def task(self):
        await asyncio.sleep(1)
        if bool(random.getrandbits(1)):
            raise Exception('Oooops a random exception!')



w = Window()
w.show()


with loop:
    loop.run_forever()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants