PyQt: How to send a stop signal into a thread where an object is running a conditioned while loop?

Question

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)

Convector

#32952974 – Teresaterese

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()) – Convector

Well, doesn't quite work, seems the sgnFinished is never fired from work method, so on_worker_done is never called on the client. – Convector

Answer 1

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_())

Arvad

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. – Noonday

@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. – Arvad

@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). – Arvad

@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? – Auriscope

@Auriscope Possibly timer will get garbage-collected when start() returns. – Arvad

Answer 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.

Davon

The OP is using PyQt4, so requestInteruption is not available (i.e. Qt >= 5.2 is required). – Arvad

Yes, requestInteruption is not available in Qt 4.8 – Convector

@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. – Arvad

But the event loop is running in the receiving thread, it is created with _thread.start() which calls run() – Convector

@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. – Arvad

@Convector 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. – Davon

@Arvad 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. – Convector

@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) – Convector