Pyside2.QtCore.QThread.terminate() Cause python to crash

brianmeasley

@jsulm This is not the problem. This is just a sample code. Please replace the URL to a downloadable file. Problems will still occur in other scenarios. For example, problems will occur when using paramiko to execute sftp to transfer files/kill threads when executing commands, but These problems do not exist in PySide6, but PySide6 does not support x86 Windows operating systems.

jsulm

@brianmeasley said in Pyside2.QtCore.QThread.terminate() Cause python to crash:

This is not the problem

If this is not the problem then post something which is actually related to your problem. Or should we guess?

brianmeasley

@jsulm

Unable to connect because the target computer actively refused it

Did you see this?
I guess you should catch the exceptions from urllib.

This is not the problem

If this is not the problem then post something which is actually related to your problem. Or should we guess?

What I actually said is that there is no problem with urllib, because the same code runs without problems in PySide6. You can run the above code and click the Start button repeatedly and it will crash (remember to replace the URL variable with a downloadable file ).
If it is a problem with urllib, the same problem should also occur in PySide6, but now the problem seems to only occur in PySide2.
The specific problem is that I found that by subclassing QThread in PySide2 and overriding the run method, when running some I/O operations that require waiting, if I want to terminate the thread immediately and execute QThread.terminate(), it will cause the entire Python program to crash.

This problem also occurs when using the paramiko library and other operations that require network I/O. I wonder if you can understand the problem I encountered?

jsulm

@brianmeasley Sorry, I will not run anything. That you can do and post the stack trace here.
Also, if you press this button repeatedly without waiting for the previous request to finish your previous QThread instance will be probably deleted by garbage collector.

brianmeasley

@jsulm

Sorry, I will not run anything. That you can do and post the stack trace here.

This is the code that runs:

import io
import sys
import time
import typing
from contextlib import suppress

import paramiko
from PySide2.QtCore import QThread
from PySide2.QtWidgets import QApplication, QWidget, QGridLayout, QPushButton


def ssh_connect(ssh_client: paramiko.SSHClient,
                hostname: str,
                port=paramiko.config.SSH_PORT,
                username: str = None,
                password: str = None,
                pkey: str = None,
                **kwargs) -> paramiko.SSHClient:
    __doc__ = paramiko.SSHClient.__doc__

    ssh_client.set_missing_host_key_policy(paramiko.AutoAddPolicy)

    if isinstance(pkey, str):
        pkey = paramiko.RSAKey.from_private_key(io.StringIO(pkey))

    hostname = hostname.strip()
    username = username.strip()

    if password == '':
        with suppress(paramiko.ssh_exception.AuthenticationException):
            ssh_client.connect(hostname, port, username=username, password='', pkey=pkey, **kwargs)
        ssh_client.get_transport().auth_none(username)
    else:
        ssh_client.connect(hostname, port, username=username, password=password, pkey=pkey, **kwargs)
    return ssh_client


class MyThread(QThread):
    def run(self) -> None:
        client = paramiko.SSHClient()
        while True:
            ssh_connect(client, '192.168.1.4', 22, 'root', ' ')
            if client.get_transport().active:
                client.close()
            time.sleep(0.1)


class Window(QWidget):
    def __init__(self, parent=None):
        super().__init__(parent)
        self._thread: typing.Optional[QThread] = None
        self.grid = QGridLayout(self)
        self.btn = QPushButton("Start", self)
        self.btn.clicked.connect(self.on_btn_clicked)
        self.grid.addWidget(self.btn)

    def on_btn_clicked(self, clicked=False):
        self.btn.setDisabled(True)
        if self._thread is not None:
            if self._thread.isRunning():
                self._thread.terminate()
            self._thread = None
            self.btn.setText("Start")
        else:
            self._thread = MyThread()
            self._thread.setTerminationEnabled(True)
            self._thread.start()
            self.btn.setText("Stop")
        self.btn.setDisabled(False)


def main():
    app = QApplication(sys.argv)
    window = Window()
    window.show()

    sys.exit(app.exec_())


if __name__ == '__main__':
    main()

After running the code, I clicked the Start button twice. The second button was used to call and kill the running QThread, but the following error occurred. This problem does not exist in PySide6:

Fatal Python error: This thread state must be current when releasing
Python runtime state: initialized

Thread 0x00007f8476872700 (most recent call first):
  File "/usr/local/lib/python3.8/dist-packages/paramiko/agent.py", line 100 in _read_all
  File "/usr/local/lib/python3.8/dist-packages/paramiko/agent.py", line 95 in _send_message
  File "/usr/local/lib/python3.8/dist-packages/paramiko/agent.py", line 434 in sign_ssh_data
  File "/usr/local/lib/python3.8/dist-packages/paramiko/auth_handler.py", line 395 in _parse_service_accept
  File "/usr/local/lib/python3.8/dist-packages/paramiko/transport.py", line 2164 in run
  File "/usr/lib/python3.8/threading.py", line 932 in _bootstrap_inner
  File "/usr/lib/python3.8/threading.py", line 890 in _bootstrap

Current thread 0x00007f84770b3700 (most recent call first):
  File "/usr/lib/python3.8/threading.py", line 306 in wait
  File "/usr/lib/python3.8/threading.py", line 558 in wait
  File "/usr/local/lib/python3.8/dist-packages/paramiko/auth_handler.py", line 240 in wait_for_response
  File "/usr/local/lib/python3.8/dist-packages/paramiko/transport.py", line 1635 in auth_publickey
  File "/usr/local/lib/python3.8/dist-packages/paramiko/client.py", line 702 in _auth
  File "/usr/local/lib/python3.8/dist-packages/paramiko/client.py", line 435 in connect
  File "/home/wo/Dev/main.py", line 34 in ssh_connect
  File "/home/wo/Dev/main.py", line 42 in run

Thread 0x00007f849f735740 (most recent call first):
  File "/home/wo/Dev/main.py", line 77 in main
  File "/home/wo/Dev/main.py", line 81 in <module>

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

SGaist

Hi,

There might have been changed between PySide6 and 2 with regard to thread termination however and in any case, thread termination like that should be the last resort when there's no other solution.

Since you thread contains an infinite loop, you should use QThread::isInterruptionRequested and QThread::requestInterruption to cleanly stop your thread.

jeremy_k

@brianmeasley said in Pyside2.QtCore.QThread.terminate() Cause python to crash:

This is the code that runs:

                self._thread.terminate()

Using QThread::terminate() is a terrible idea:
Warning: This function is dangerous and its use is discouraged. The thread can be terminated at any point in its code path. Threads can be terminated while modifying data. There is no chance for the thread to clean up after itself, unlock any held mutexes, etc.

On Windows, it appears to be implemented with TerminateThread():
TerminateThread is a dangerous function that should only be used in the most extreme cases. You should call TerminateThread only if you know exactly what the target thread is doing, and you control all of the code that the target thread could possibly be running at the time of the termination.

The QThread unix implementation uses pthread_cancel(). The documentation I've seen doesn't carry as bold of a caveat, but it should.

After running the code, I clicked the Start button twice. The second button was used to call and kill the running QThread, but the following error occurred. This problem does not exist in PySide6:
Fatal Python error: This thread state must be current when releasing
Python runtime state: initialized

That looks like the python interpreter complaining that its internal state is not as expected, as might occur if the thread of execution was unexpectedly terminated...

brianmeasley

@SGaist @jeremy_k

thank you for your reply,
It's true that using QThread.terminate() to kill a thread is not a good idea.
I would like to ask a related question. If a thread is executing a long-running task and the task puts the thread in a blocked state, what should I do if I want to terminate the blocked thread immediately?
For example, in the ssh_connect code above, if the while loop in the run method is removed and a connection is made, a crash will still occur. If this is placed in an SSH terminal program/SFTP file manager, the user wants to stop logging in/refresh the directory immediately. How to implement the operation?

brianmeasley

I found the solution! I checked the official binding source code of PySide, and in fact PySide2-5.15.2.1 and later versions fix this bug.

JonB

@brianmeasley said in Pyside2.QtCore.QThread.terminate() Cause python to crash:

If a thread is executing a long-running task and the task puts the thread in a blocked state, what should I do if I want to terminate the blocked thread immediately?

You may have already found your solution. But just to pick up on this. If you expect to want the ability to interrupt/terminate a thread what you are supposed to do is have it run an event loop. Then you can either send it a Qt signal or PySide2.QtCore.QThread.requestInterruption() and isInterruptionRequested(). I think it's worth knowing about these, maybe for other cases if it does not fit yours.

If what it calls in some external blocking operation and you cannot be running an event (or other) loop checking for "interruption request" then I agree you are in trouble. And you may have to terminate the thread. But this may indeed do bad things to the state of your execution, system resource usage or whatever. But then you would equally be in a pickle for this whether you used a thread or not.

SGaist

The usual solution, when you have a long running operation is to check at regular points in your code for termination request. It's not always possible but it's a pretty standard solution. Then, if you really have to kill the thread, don't restart it, create a new one that will be clean.

jeremy_k

@JonB said in Pyside2.QtCore.QThread.terminate() Cause python to crash:

If what it calls in some external blocking operation and you cannot be running an event (or other) loop checking for "interruption request" then I agree you are in trouble. And you may have to terminate the thread. But this may indeed do bad things to the state of your execution, system resource usage or whatever. But then you would equally be in a pickle for this whether you used a thread or not.

Make it an external process. If it must be unceremoniously terminated, leave no part of the process running in a possibly corrupted state.

JonB

@jeremy_k said in Pyside2.QtCore.QThread.terminate() Cause python to crash:

Make it an external process.

If you can. You know there are plenty of circumstances where this may not be suitable/possible.

jeremy_k

@JonB said in Pyside2.QtCore.QThread.terminate() Cause python to crash:

@jeremy_k said in Pyside2.QtCore.QThread.terminate() Cause python to crash:

Make it an external process.

If you can. You know there are plenty of circumstances where this may not be suitable/possible.

Use explicit communication rather than relying on pointers and a shared address space. If a thread is killed in the middle of modifying data in that address space, you can't count on that data having sane values. The same applies to modifying without explicit signaling/barriers. I am unaware of any systems that reorder communication within a single i/o channel. That makes earlier data potentially usable even with later data is absent.

Excluding resource limited environments where the extra communication exceeds the available time or memory, I don't know of situations where a second thread can do something that a second process can't. Different interfaces may be required.

This is a monolithic versus microkernel discussion, except in user space.

JonB

@jeremy_k said in Pyside2.QtCore.QThread.terminate() Cause python to crash:

I don't know of situations where a second thread can do something that a second process can't.

Our experiences differ them. But let's leave it at that, may have nothing to with OP's situation. I have never disagreed that terminating is bad, and that external process might be preferable. Let's see how OP gets on with that.