Search code examples
pythonmultithreadingqt5qthreadpyside2

PySide2: Proper way to clean up grandchild threads


I'm using PySide2 to write a multithreaded application. I have a Controller object already running in its own thread so that it does not block the GUI thread. This Controller needs to spin up multiple Worker threads that perform work continuously. I can manually start and stop these grandchild threads correctly with a signal, but I'm having trouble cleaning them up with signals on application exit.

Here's a toy example that replicates my problem:

import sys

import shiboken2
from PySide2.QtCore import QObject, QThread
from PySide2.QtWidgets import QApplication, QPushButton


class Grandchild(QObject):
    def __init__(self, parent=None):
        super(Grandchild, self).__init__(parent)
        print('Grandchild()')

    def __del__(self):
        print('~Grandchild()')


class Child(QObject):
    _thread = None
    _worker = None

    def __init__(self, parent=None):
        super(Child, self).__init__(parent)
        print('Child()')

    def __del__(self):
        print('~Child()')
        if shiboken2.isValid(self._thread):
            self.stop_thread()

    def start_thread(self):
        print('Starting grandchild thread')
        self._thread = QThread(self)
        self._worker = Grandchild()
        self._worker.moveToThread(self._thread)
        self._thread.finished.connect(self._worker.deleteLater)
        self._thread.start()

    def stop_thread(self):
        print('Stopping grandchild thread')
        self._thread.quit()
        self._thread.wait()

    def toggle_thread(self):
        if self._thread and self._thread.isRunning():
            self.stop_thread()
        else:
            self.start_thread()


class Parent(QPushButton):
    _thread = None
    _worker = None

    def __init__(self, parent=None):
        super(Parent, self).__init__(parent)
        print('Parent()')
        self.setText('Start Grandchild')

        self._thread = QThread(self)
        self._worker = Child()
        self._worker.moveToThread(self._thread)
        self._thread.finished.connect(self._worker.deleteLater)
        self._thread.start()

        self.clicked.connect(self.on_push)
        self.clicked.connect(self._worker.toggle_thread)

    def __del__(self):
        print('~Parent()')
        if shiboken2.isValid(self._thread):
            self._thread.quit()
            self._thread.wait()

    def on_push(self):
        if self.text() == 'Start Grandchild':
            self.setText('Stop Grandchild')
        else:
            self.setText('Start Grandchild')


def main():
    app = QApplication(sys.argv)

    widget = Parent()
    widget.show()
    sys.exit(app.exec_())


if __name__ == "__main__":
    main()

In this example, starting and stopping the threads using the button works fine. If I start and stop the grandchild with the button and then close the application, all destructors call correctly and the application exits correctly. If I only start the grandchild and close the application, I get:

Parent()
Child()
Starting grandchild thread
Grandchild()
QThread: Destroyed while thread is still running
~Parent()

Process finished with exit code 134 (interrupted by signal 6: SIGABRT)

My first thought was to modify the Parent like so:

    stop_child = Signal()

    def __init__(self):
        self.stop_child.connect(self._worker.stop_thread)


    def __del__(self):
        print('~Parent()')
        self.stop_child.emit()
        if shiboken2.isValid(self._thread):
            self._thread.quit()
            self._thread.wait()

As is, this does not affect the problem at all. If I put a breakpoint in Child.stop_thread, I can see that it never gets executed before the SIGABRT gets thrown. If I put a breakpoint inside Parent.__del__, execution stops as expected. If I resume execution, it does stop at my Child.stop_thread breakpoint. So something about pausing in the destructor is allowing the signal to get caught in the Child thread? Anyway, this doesn't function without breakpoints, so that doesn't work.

I removed all of that and did something that seems really dumb (in the long-term):

    # Parent
    def __del__(self):
        print('~Parent()')
        self._worker.stop_thread() # Call the instance fn directly
        if shiboken2.isValid(self._thread):
            self._thread.quit()
            self._thread.wait()

And of course, it works:

Parent()
Child()
Starting grandchild thread
Grandchild()
~Parent()
Stopping grandchild thread
~Child()
~Grandchild()

Process finished with exit code 0

It seems like it would be a really terrible idea to call an instance function for an object that lives in another thread. I'm assuming it works because my stop_child function (in both this toy and in my actual code) is implicitly thread-safe.

So this leads me to my questions:

  • What is the correct way to cleanup grandchildren threads when the primary thread is shutting down?
  • What if, as in this example, I want the existence of those grandchildren threads abstracted behind a member object?
  • Shouldn't the signals/slots approach have worked?

Solution

  • After more experimentation, it appears as though the signals/slots approach will work if you make the connection a Qt.BlockingQueuedConnection.

    self.stop_child.connect(self._worker.stop_thread, Qt.BlockingQueuedConnection)
    

    This means that the main thread blocks until the signal is delivered, which might not be desirable if the stop_child signal is used during normal runtime operation. It seems to be alright in the main widget's destructor, though.

    The complete working example follows:

    import sys
    
    import shiboken2
    from PySide2.QtCore import QObject, QThread, Signal, Qt
    from PySide2.QtWidgets import QApplication, QPushButton
    
    
    class Grandchild(QObject):
        def __init__(self, parent=None):
            super(Grandchild, self).__init__(parent)
            print('Grandchild()')
    
        def __del__(self):
            print('~Grandchild()')
    
    
    class Child(QObject):
        _thread = None
        _worker = None
    
        def __init__(self, parent=None):
            super(Child, self).__init__(parent)
            print('Child()')
    
        def __del__(self):
            print('~Child()')
            if shiboken2.isValid(self._thread):
                self.stop_thread()
    
        def start_thread(self):
            print('Starting grandchild thread')
            self._thread = QThread(self)
            self._worker = Grandchild()
            self._worker.moveToThread(self._thread)
            self._thread.finished.connect(self._worker.deleteLater)
            self._thread.start()
    
        def stop_thread(self):
            print('Stopping grandchild thread')
            self._thread.quit()
            self._thread.wait()
    
        def toggle_thread(self):
            if self._thread and self._thread.isRunning():
                self.stop_thread()
            else:
                self.start_thread()
    
    
    class Parent(QPushButton):
        _thread = None
        _worker = None
    
        stop_child = Signal()
    
        def __init__(self, parent=None):
            super(Parent, self).__init__(parent)
            print('Parent()')
            self.setText('Start Grandchild')
    
            self._thread = QThread(self)
            self._worker = Child()
            self._worker.moveToThread(self._thread)
            self._thread.finished.connect(self._worker.deleteLater)
            self._thread.start()
    
            self.clicked.connect(self.on_push)
            self.clicked.connect(self._worker.toggle_thread)
            self.stop_child.connect(self._worker.stop_thread, Qt.BlockingQueuedConnection)
    
        def __del__(self):
            print('~Parent()')
            self.stop_child.emit()
            if shiboken2.isValid(self._thread):
                self._thread.quit()
                self._thread.wait()
    
        def on_push(self):
            if self.text() == 'Start Grandchild':
                self.setText('Stop Grandchild')
            else:
                self.setText('Start Grandchild')
    
    
    def main():
        app = QApplication(sys.argv)
    
        widget = Parent()
        widget.show()
        sys.exit(app.exec_())
    
    
    if __name__ == "__main__":
        main()