6

I'm doing some multi-threading. I have a worker class with a work method, which I send into a separate QThread. The work method has a conditioned while loop inside. I want to be able to send a signal to the worker object to stop it (changing the _running condition to false). This will cause the while loop to exit, and a finished signal to be sent from the worker object (which is connected to the quit slot of the worker's thread).

The false condition is sent to the worker object via a signal, but it is never received, which I believe is because the while loop blocks the event-loop of its thread. Even if I put QCoreApplication.processEvents() inside the while loop, nothing happens. Where is the problem? Why isn't the signal processed? (Notice that the print statement in the stop slot on the Worker is never executed - but the weird thing is, the thread does seem to stop in a wrong way).

Here is the code:

import time, sys
from PyQt4.QtCore  import *
from PyQt4.QtGui import *

class Worker(QObject):
    sgnFinished = pyqtSignal()

    def __init__(self, parent):
        QObject.__init__(self, parent)

        self._running = True

    @pyqtSlot()
    def stop():
        print 'stop signal received, switching while loop condition to false'
        self._running = False

    @pyqtSlot()    
    def work(self):
        while self._running:                 #this blocks the thread, if changed to an if clause, thread finishes as expected!
            QCoreApplication.processEvents() #this doesn't help!
            time.sleep(0.1)
            print 'doing work...'

        #do some cleanup here, then signal the worker is done
        self.sgnFinished.emit()


class Client(QObject):
    sgnStop = pyqtSignal()

    def __init__(self, parent):
        QObject.__init__(self, parent)

        self._thread = None
        self._worker = None

    def toggle(self, enable):
        if enable:
            if not self._thread:
                self._thread = QThread()

            self._worker = Worker(None)
            self._worker.moveToThread(self._thread)

            self._worker.sgnFinished.connect(self.on_worker_done)
            self.sgnStop.connect(self._worker.stop)

            self._thread.started.connect(self._worker.work)
            self._thread.start()
        else:
            print 'sending stop signal to the worker object'
            self.sgnStop.emit() #send a queuedconnection type signal to the worker, because its in another thread

    @pyqtSlot() 
    def on_worker_done(self):
        print 'workers job was interrupted manually'
        self._thread.quit()
        #self._thread.wait() not sure this is neccessary

if __name__ == '__main__':
    app = QCoreApplication(sys.argv)

    client = Client(None)
    client.toggle(True)
    raw_input('Press something')
    client.toggle(False)
ekhumoro
  • 115,249
  • 20
  • 229
  • 336
U2ros
  • 243
  • 2
  • 11
  • http://stackoverflow.com/questions/32952474/non-blocking-worker-interrupt-file-copy – dtech Oct 11 '16 at 12:41
  • I found a solution that i don't understand. Replacing stop signal connection declaration with this (adding the handler as a lambda expression) fixes the problem: self.sgnStop.connect(lambda: self._worker.stop()) – U2ros Oct 11 '16 at 13:04
  • Well, doesn't quite work, seems the sgnFinished is never fired from work method, so on_worker_done is never called on the client. – U2ros Oct 11 '16 at 13:08

2 Answers2

5

There are two main problems in your example:

Firstly, you are emitting a signal to stop the worker, but since the signal is cross-thread, it will be posted in the receiver's event-queue. However, the worker is running a blocking while-loop, so pending events cannot be processed. There are a few ways to work around this, but probably the simplest is to simply call the worker's stop method directly instead of using a signal.

Secondly, you are not explicitly running an event-loop in the main thread, so cross-thread signals sent from the worker cannot be queued. More importantly, though, there is also nothing to stop the program exiting after the user presses a key - so the client and worker will be immediately garbage-collected.

Below is a re-written version of your example which fixes all the issues:

import time, sys
from PyQt4.QtCore  import *
from PyQt4.QtGui import *

class Worker(QObject):
    sgnFinished = pyqtSignal()

    def __init__(self, parent):
        QObject.__init__(self, parent)
        self._mutex = QMutex()
        self._running = True

    @pyqtSlot()
    def stop(self):
        print 'switching while loop condition to false'
        self._mutex.lock()
        self._running = False
        self._mutex.unlock()

    def running(self):
        try:
            self._mutex.lock()
            return self._running
        finally:
            self._mutex.unlock()

    @pyqtSlot()
    def work(self):
        while self.running():
            time.sleep(0.1)
            print 'doing work...'
        self.sgnFinished.emit()

class Client(QObject):
    def __init__(self, parent):
        QObject.__init__(self, parent)
        self._thread = None
        self._worker = None

    def toggle(self, enable):
        if enable:
            if not self._thread:
                self._thread = QThread()

            self._worker = Worker(None)
            self._worker.moveToThread(self._thread)
            self._worker.sgnFinished.connect(self.on_worker_done)

            self._thread.started.connect(self._worker.work)
            self._thread.start()
        else:
            print 'stopping the worker object'
            self._worker.stop()

    @pyqtSlot()
    def on_worker_done(self):
        print 'workers job was interrupted manually'
        self._thread.quit()
        self._thread.wait()
        if raw_input('\nquit application [Yn]? ') != 'n':
            qApp.quit()

if __name__ == '__main__':

    # prevent some harmless Qt warnings
    pyqtRemoveInputHook()

    app = QCoreApplication(sys.argv)

    client = Client(None)

    def start():
        client.toggle(True)
        raw_input('Press something\n')
        client.toggle(False)

    QTimer.singleShot(10, start)

    sys.exit(app.exec_())
ekhumoro
  • 115,249
  • 20
  • 229
  • 336
  • 1
    **This is insanity.** Never "simply call the worker's `stop` method directly instead of using a signal" unless you *really* think you know what you're doing. In this case, you don't, because you explicitly lock the already thread-safe `stop` slot with a low-level mutual exclusion primitive – which completely defeats the purpose of using a high-level signal-slot connection in the first place! Slots are robust. Locks are *not*. Slots obsolete locks. You use slots entirely so that you *don't* have to use locks. If you're going to use locks (which you shouldn't), there's no reason to use slots. – Cecil Curry Mar 29 '18 at 05:31
  • @CecilCurry. Cross-thread signals are thread-safe, but, by themselves, slots aren't, Using the above script, if you put `print int(QThread.currentThreadId())` in the `stop` and `work` slots, you will see that `slot` is executed in the main thread. That is why I used a mutex. The `stop` slot isn't called by a signal, so the thread-safe guarantee doesn't apply. The reason I didn't use a signal, is because the `work` slot is running a blocking while-loop which prevents the worker thread processing it's event-queue. Cross-thread signals require a running event-loop in the receiving thread. – ekhumoro Mar 29 '18 at 18:58
  • @CecilCurry. PS: I meant `stop` is executed in the main thread. PPS: I'm sure you know your stuff, but please test my claims by experimenting with the script in my answer. I'm sure there are other ways of solving the OPs problems, but I don't believe there is anything wrong with what I wrote. The other answer by Kevin Krammer (and the comments to it) are all in agreement with my own answer, and eventually propose the same solution (which is specific to pyqt4). – ekhumoro Mar 29 '18 at 19:07
  • @ekhumoro, hope this won't be regarded as out of context: I changed `raw_input()` to a `QTimer(4000,lambda: client.toggle(False)` and it works. But changing to a `timer=QTimer(); timer.timeout.connect(lambda: client.toggle(False)); timer.start(4000)` doesn't work. What's wrong with the latter? – Jason Jan 29 '19 at 08:12
  • @Jason Possibly `timer` will get garbage-collected when `start()` returns. – ekhumoro Jan 29 '19 at 17:14
2

Cross thread signal/slot connections require a running event loop in the thread of the receiver object.

In your case there is an event loop in the second thread and it is running, but it is at all times executing your work method and never returns from there.

So all slot invocation events are stuck in the event loop's event queue.

If you want to hack around this, like you attempted with QCoreApplication.processEvents you could try getting the thread's eventDispatcher and calling its processEvent.

If you only need to end the worker, you could call the thread's requestInteruption and instead of checking for self._running you check for the thread's isInterruptionRequested.

ekhumoro
  • 115,249
  • 20
  • 229
  • 336
Kevin Krammer
  • 5,159
  • 2
  • 9
  • 22
  • The OP is using PyQt4, so `requestInteruption` is not available (i.e. Qt >= 5.2 is required). – ekhumoro Oct 11 '16 at 12:43
  • Yes, requestInteruption is not available in Qt 4.8 – U2ros Oct 11 '16 at 13:12
  • @U2ros. The first sentence of this answer is the most relevant to your problem. Your example just won't work properly unless there's a running event-loop in the main thread. – ekhumoro Oct 11 '16 at 13:15
  • But the event loop is running in the receiving thread, it is created with _thread.start() which calls run() – U2ros Oct 11 '16 at 13:22
  • @U2ros. You are sending signals between the worker thread and the main thread. All cross-thread signals are queued by default. So you need a running event-loop in **both** threads. – ekhumoro Oct 11 '16 at 13:34
  • @U2ros the loop is currently executing one method invokation, `work` and that has an endless loop. So unless that loop ends or breaks the call to `work` does not return, thus no new events can be processed. If the Qt version is to old to have support for `requestInterruption` then you can easily implement something similar yourself using a boolean flag, a mutex and two methods, one to set and one to read the member under mutex protection. – Kevin Krammer Oct 11 '16 at 22:20
  • @ekhumoro I subclassed a QCoreApplication, startings its event loop by calling exec_ in main(), and doing the client logic from its constructor. It kind of works, but the worker thread is destroyed as soon as i change the running condition to false. – U2ros Oct 12 '16 at 08:13
  • @KevinKrammer same as above, but i in this case call stop() directly not via a signal. changing of _running inside stop is wrapped in a mutex lock, and reading its value in the work method too. Result is the same, thread dies immediately after _running is changed. (sgnFinished is never emited from work method) – U2ros Oct 12 '16 at 08:19