How to stop QThread "gracefully"?

Question

I have got some help to do the following code. But I need to break out from this loop which is in the Worker thread, so when I exit from the application, it does not crash. At the moment QThread is still running when I quit from the app. If I use break statement it works, but then I cannot do another search for hosts, because the loop has been closed entirely. I have tried several ways to do it, but no luck. I am new to programming.

class Worker(QThread):
    found = Signal(str)
    notFound = Signal(str)
    def __init__(self):
        QThread.__init__(self)
        self.queue = Queue()

    def run(self):
        while True:
            hostname = self.queue.get()
            output_text = collect_host_status(hostname)
            for i in output_text:
                if "not found" in i:
                    self.notFound.emit(i.replace(" not found", ""))
                else:
                    self.found.emit(i)

    def lookUp(self, hostname):
        self.queue.put(hostname)


class MainWindow(QMainWindow):
    def __init__(self):
        # ...
        self.ui.pushButton_2.clicked.connect(self.buttonclicked)

        self.thread = Worker()
        self.thread.found.connect(self.ui.textEdit_2.append)
        self.thread.notFound.connect(self.ui.textEdit_3.append)
        self.thread.start()

    def buttonclicked(self):
        if self.ui.textEdit.toPlainText():
            self.thread.lookUp(self.ui.textEdit.toPlainText())

Here is the code for the collect host status:

def get_brss_host_status(host):
    
    host = host.lower()
    api_url = 'https://some.system.with/api/'
    host_search = 'status/host/{}?format=json'.format(host)

    
    r = requests.get(api_url + host_search, auth=(loc_brss_user, loc_brss_passwd))
    request_output = r.text
    if request_output == '{"error":"Not Found","full_error":"Not Found"}':
        host2 = host.upper()
        host_search2 = 'status/host/{}?format=json'.format(host2)
        r2 = requests.get(api_url + host_search2, auth=(loc_brss_user, loc_brss_passwd))
        request_output2 = r2.text
        # print('Debug request_output2', request_output2)
        if request_output and request_output2 == '{"error":"Not Found","full_error":"Not Found"}':
            output_string = host + " not found"
        else:
            output_string = host2

    else:
        output_string = host
    
    return output_string


def collect_host_status(hosts):
    
    hosts_list = list(hosts.split("\n"))
        
    status_list = []
    for i in hosts_list:
        
        host = get_brss_host_status(i)
        status_list.append(host)
    
    return status_list

We don't know what your `collect_host_status` **exactly** does, and only knowing how it behaves we could try to give an appropriate answer. For instance: if the function cannot be stopped in any way before completion, you probably need to add a message box that tells the user that it has to wait until it finishes (which could take a lot of time, again, we don't know that). — musicamante, Mar 05 '21 at 13:27
`collect_host_status` is imported from another py file. This is connecting to the server's API. It is making a list by the entered hostnames of the GUI textEdit field, and checking these hosts if they are present or not. — Oborzil, Mar 05 '21 at 13:49
@Oborzil Do `self._running = True; while self._running:` and add a method `def stop(self): self._running = False`. — ekhumoro, Mar 05 '21 at 13:51
@Oborzil besides the possible solution provided by ekhumoro, I asked you "what it exactly does" not "describe its purpose", meaning that you should probably show us its code or a [minimal, reproducible example](https://stackoverflow.com/help/minimal-reproducible-example) of it: the problem is that if that process could potentially wait indefinitely, even with the solution above you'd probably have problems anyway, even if you use the thread's `wait()` (which will block the UI until the thread finally exits its loop). — musicamante, Mar 05 '21 at 14:03
@musicamante What problems are you referring to? There are lots of working solutions on SO that use this approach (sometimes with the addition of a mutex). If `collect_host_status` blocks indefinitely, multi-threading is obviously the wrong solution altogether, and multi-processing should be used instead. — ekhumoro, Mar 06 '21 at 17:10
@ekhumoro the general solution is usually fine, the problem is usually when there's no direct control over the waiting process, and the wait could take too much time. A network request could be one case if, for example, some unexpected network problems that may not *fail* the request, but delay it. I know it's not a "technical" problem of the implementation, but still might require more attention. — musicamante, Mar 06 '21 at 18:15
Ok. Now I am a bit confused. Shall I use multiprocess instead? Or is there a way to keep this code and solve it somehow? — Oborzil, Mar 06 '21 at 18:30

score 1 · Accepted Answer · answered Mar 06 '21 at 18:38

The base solution, as suggested in the comments by @ekhumoro, is to use a simple flag in the while loop, which will ensure that as soon as the cycle restarts it exits if the condition is not respected.

Some care should be used, though, for two important aspects:

using the basic get() of Queue makes the cycle wait undefinitely;
the function in the example (a network request) might be delayed for some time if any network problem occurs (temporary network issues, etc);

To correctly solve these issues, the following modifications should be done:

get() should use a timeout, so that it allows exiting the cycle even when no request is being queued; as an alternative, you can unset the "running" flag, add anything to the queue and check for the flag before proceeding: this ensures that you don't have to wait for the queue get() timeout;
the network requests should have a minimum timeout too;
they should be done individually from the thread, and not grouped, so that the thread can exit if the requested host list is too big and you want to quit while doing look ups;

from queue import Queue, Empty

class Worker(QThread):
    found = Signal(str)
    notFound = Signal(str)
    def __init__(self):
        QThread.__init__(self)
        self.queue = Queue()

    def run(self):
        self.keepRunning = True
        while self.keepRunning:
            hostList = self.queue.get()
            if not self.keepRunning:
                break

            # otherwise:
            # try:
            #     hostList = self.queue.get(timeout=1)
            # except Empty:
            #     continue

            for hostname in hostList.splitlines():
                if not self.keepRunning:
                    break
                if hostname:
                    output_text = get_brss_host_status(hostname)
                    if output_text is None:
                        continue
                    if "not found" in output_text:
                        self.notFound.emit(output_text.replace(" not found", ""))
                    else:
                        self.found.emit(output_text)

    def stop(self):
        self.keepRunning = False
        self.queue.put(None)

    def lookUp(self, hostname):
        self.queue.put(hostname)

And in the get_brss_host_status, change the following:

def get_brss_host_status(host):
    
    host = host.lower()
    api_url = 'https://some.system.with/api/'
    host_search = 'status/host/{}?format=json'.format(host)


    try:
        r = requests.get(api_url + host_search, 
            auth=(loc_brss_user, loc_brss_passwd), 
            timeout=1)
    except Timeout:
        return

    # ...

Thanks for your effort, I have tried your solution which you mentioned here. It works as expected but quiting from the GUI still leads to crash. QThread is still running :( — Oborzil, Mar 06 '21 at 19:08
Ohh yeah missed to call wait(). Works like a charm! Thanks very much for the help! You are awesome! — Oborzil, Mar 06 '21 at 19:33

How to stop QThread "gracefully"?

1 Answers1

Linked

Related