[FIXED] Why pyqtsignal failed to trigger normally?

Issue

I am a pyqt newbie and I am using pyqtsignal in my application and don't know why it is not working properly.I don't understand why this is happening, the rest of pyqtsignal in the program works fine, I'm wondering if I'm missing something?

When I click the stop button, the program does not stop. If I call runner.stop() directly, the program can exit normally.

so i'm very confused. Below is my code:

class Runner(QObject):
    stopped = pyqtSignal()

    def __init__(self, name):
        super().__init__()
        self.name = name
        self.run_flag = True

    def start(self):
        print("start", self.run_flag)
        while self.run_flag:
            self.timeoutSlot()
            time.sleep(2)

    def stop(self):
        logging.warning("stop called.")
        self.run_flag = False
        self.stopped.emit()

    def timeoutSlot(self):
        # long time work
        time.sleep(10)
        logging.info(f"{self.name} is running")


class MainWindow(QMainWindow):
    stopSignal = pyqtSignal()

    def __init__(self):
        super().__init__()
        self._central_widget = QWidget()
        self._central_layout = QVBoxLayout()
        self.start_btn = QPushButton("start")
        self.stop_btn = QPushButton("stop")
        self._central_layout.addWidget(self.start_btn)
        self._central_layout.addWidget(self.stop_btn)
        self._central_widget.setLayout(self._central_layout)
        self.setCentralWidget(self._central_widget)
        self.start_btn.clicked.connect(self.start)
        self.stop_btn.clicked.connect(self.stop)

        self.connects = []
        self.runners = []

    def start(self):
        for i in range(2):
            setattr(self, f"thread_{i}", QThread())
            setattr(self, f"runner_{i}", Runner(f"runner_{i}"))
            thread = getattr(self, f"thread_{i}")
            runner = getattr(self, f"runner_{i}")
            assert isinstance(thread, QThread)
            assert isinstance(runner, Runner)
            runner.moveToThread(thread)
            thread.started.connect(runner.start)

            conn = self.stopSignal.connect(runner.stop)
            self.connects.append(conn)

            runner.stopped.connect(thread.quit)
            runner.stopped.connect(runner.deleteLater)
            self.runners.append(runner)
            thread.finished.connect(thread.deleteLater)
            thread.start()

    def stop(self):
        # blow can work normally
        # for runner in self.runners:
        #     runner.stop()
        # self.runners = []
        print("stop btn clicked!")
        
        # can't work
        self.stopSignal.emit()
        # for conn in self.connects:
        #     self.stopSignal.disconnect(conn)


if __name__ == '__main__':
    app = QApplication(sys.argv)
    win = MainWindow()
    win.show()
    sys.exit(app.exec())

Solution

The signal does work, the problem is that since it's connected to a thread, its connection is a QueuedConnection. This means that the connected slots will only be called as soon as the receiver thread allows so.

You've connected the started signal of the thread to the start function, which immediately run a blocking loop, preventing the QThread event loop to go further until that function returns control to it. Since queued signals can only be processed by their event loop, that never happens.

There are two possible solutions for this specific case:

1. directly call the stop() function of each runner;
2. use a DirectConnection for the stopSignal;

In the first case, just iterate through all runners:

    def stop(self):
        for runner in self.runners:
            runner.stop()

Alternatively:

    def start(self):
        for i in range(2):
            ...
            conn = self.stopSignal.connect(runner.stop, Qt.DirectConnection)
            ...

Note that your usage of getattr and setattr is unnecessarily convoluted: just create a local variable and then call setattr, then get rid of the assert, which makes no sense since you just created those objects.

Also, that assumes that you always have persistent references that may be overwritten when start() is called again, with the result of threads being destroyed while running.

A better approach is to use a list for both threads and runners, so that you can eventually iterate again existing objects and decide what to do (restart the threads, stop them, etc.).

Answered By - musicamante