Threads stopping once the funciton ends

[Joe Smith]

I am making a port scanner app. I have a function which collects all data once the button is clicked and creates threads from a scanning function. Here is a snippet:

if (SCAN_TYPE == "TCP") {
        for (int i = 0; i < THREAD_NUMBER; i++) {
            int end = (i == THREAD_NUMBER - 1) ? STOP_PORT : PORT_CHUNK*(i+1);

            TcpScanThread* tcpThread = new TcpScanThread(START_PORT+(i*PORT_CHUNK), end, TARGET_IPV4, CONNECTION_TIMEOUT);
            connect(tcpThread, &QThread::finished, tcpThread, &QObject::deleteLater);
            tcpThread->start();
            activeThreads.append(tcpThread);
        }
    }

this creates and starts threads, which run as intended. But once when the function finishes (button click function which calls above code), all threads just stop and no ports are scanned anymore.

I tried storing threads inside a QList so that they would last, but it didnt work:
mainwindow.h:

#ifndef MAINWINDOW_H
#define MAINWINDOW_H

#include "tcpScanThread.h"
#include <QMainWindow>

QT_BEGIN_NAMESPACE
namespace Ui {
class MainWindow;
}
QT_END_NAMESPACE

class MainWindow : public QMainWindow
{
    Q_OBJECT

public:
    MainWindow(QWidget *parent = nullptr);
    ~MainWindow();

private slots:
    void on_scan_start_clicked();

    void on_scan_type_currentTextChanged(const QString &scanType);

private:
    Ui::MainWindow *ui;
    QList<TcpScanThread*> activeThreads;
};
#endif // MAINWINDOW_H

If I just use regular c++ threads, the app of course crashes because it has to wait. But if it doesn't wait, the threads are gone.
I want it to create threads which don't need block the main thread and are not destroyed once the function ends.
Here is also the complete function for scan button:

void MainWindow::on_scan_start_clicked()
{
    const int START_PORT = ui->start_port_inp->text().toInt();
    const int STOP_PORT = ui->end_port_inp->text().toInt();

    QByteArray TARGET_IPV4_Byte = ui->host_inp->text().toLocal8Bit();
    const char* TARGET_IPV4 = TARGET_IPV4_Byte.constData();
    std::string ipv4Buffer(TARGET_IPV4);
    TARGET_IPV4_HEADER = ipv4Buffer.data();

    const int CONNECTION_TIMEOUT = ui->connectionTO_inp->text().toInt();
    const std::string SCAN_TYPE = ui->scan_type->currentText().toStdString();
    const int THREAD_NUMBER = ui->threadNum_inp->text().toInt();


    const int PORT_CHUNK = (STOP_PORT - START_PORT + 1) / THREAD_NUMBER;

    if (SCAN_TYPE == "TCP") {
        for (int i = 0; i < THREAD_NUMBER; i++) {
            int end = (i == THREAD_NUMBER - 1) ? STOP_PORT : PORT_CHUNK*(i+1);

            TcpScanThread* tcpThread = new TcpScanThread(START_PORT+(i*PORT_CHUNK), end, TARGET_IPV4, CONNECTION_TIMEOUT);
            connect(tcpThread, &QThread::finished, tcpThread, &QObject::deleteLater);
            tcpThread->start();
            activeThreads.append(tcpThread);
        }
    }
}

[SGaist]

Hi and welcome to devnet,

What is the implementation of TcpScanThread ?

[Pl45m4]

@Joe-Smith

Run your debugger and check at what point exactly the threads stop and what state they are in.
Unless I'm missing something from the first glance your approach should work.
The obvious would have been "thread pointer/object goes out of scope and is destroyed" which should not be the case here since you heap allocate your threads and store them in your list.

Architecture-wise, because you are splitting the whole scan task into chunks and assign the work to dedicated threads, I think it's worth looking at QThreadPool :)

• https://doc.qt.io/qt-6/qthreadpool.html

[Joe Smith]

@SGaist
I am trying to send it, but it keeps getting flagged as spam. So I will just send the main part:

class TcpScanThread : public QThread {
    Q_OBJECT
public:
    TcpScanThread(const int START_PORT, const int STOP_PORT, const char* TARGET_IPV4, const int CONNECTION_TIMEOUT)
        :START_PORT(START_PORT), STOP_PORT(STOP_PORT), TARGET_IPV4(TARGET_IPV4), CONNECTION_TIMEOUT(CONNECTION_TIMEOUT) {}

protected:
    void run() override {
        tcpScan(START_PORT, STOP_PORT, TARGET_IPV4, CONNECTION_TIMEOUT);
    }
private:
    const int START_PORT;
    const int STOP_PORT;
    const char* TARGET_IPV4;
    const int CONNECTION_TIMEOUT;
};

[Joe Smith]

@Pl45m4 Hi.
The process of how everything happens is as follows:

• Threads are created

• Right before the function exits, the threads just manage to print out 2 open ports

• Function ends and all threads just stop/are destroyed

I tested this by putting a 20s sleep timer at the end of a function:

if (SCAN_TYPE == "TCP") {
        for (int i = 0; i < THREAD_NUMBER; i++) {
            int end = (i == THREAD_NUMBER - 1) ? STOP_PORT : PORT_CHUNK*(i+1);

            TcpScanThread* tcpThread = new TcpScanThread(START_PORT+(i*PORT_CHUNK), end, TARGET_IPV4, CONNECTION_TIMEOUT);
            connect(tcpThread, &QThread::finished, tcpThread, &QObject::deleteLater);
            tcpThread->start();
            activeThreads.append(tcpThread);
        }
    }
QThread::msleep(20000);

And of course it crashed the app, but the threads were still working, and printed out all ports, before it crashed.

[Joe Smith]

Okay everyone, I fount the problem.

QByteArray TARGET_IPV4_Byte = ui->host_inp->text().toLocal8Bit();
const char* TARGET_IPV4 = TARGET_IPV4_Byte.constData();

Target IP is getting destroyed when the function exits. And so all thread that use it cannot function.
I can confirm this by replacing this variable with a static string:

TcpScanThread* tcpThread = new TcpScanThread(START_PORT+(i*PORT_CHUNK),
 end, "123.123.123.123", CONNECTION_TIMEOUT);

And it works. I don't even have to store threads in a QList.
At the end I just moved TARGET_IPV4_Byte and TARGET_IPV4 into the mainwindow.h and just set them from the function.

[SGaist]

@Joe-Smith rather than moving char* around, you should simply store your TARGET_IPV4 variable as a QByteArray and only pass the pointer to char at the last moment to tcpScan.

On a side note, I would recommend not using variables name with all upper cased letters. While it's allowed, many if not most people use them for macro names and global constants so it makes your code harder to read/reason about especially when your snippets do not provide the full context.

[Joe Smith]

@SGaist I thought that it's a standard for any variables to be written in upper case if they are constant. But, it is true that my variables here are not global. So it's just for global constants?

[SGaist]

@Joe-Smith well... they are const in the sense that you won't be able to change them afterward but you still set them dynamically.

I would recommend that yes, full upper case only for global const variables.

[Joe Smith]

@SGaist
Thank you. I really appreciate all info.
Have a nice day.