views:

234

answers:

2

Following thread code runs 2058 times, after that it crashes. Can somebody help me figure out why? The idea of the program is create some class in main thread, pass it to worker thread, thread fills needed data and pass data back to main thread. This example crashes after 2058 runs, however it should go indefinately. I've run it 20 times, always the same number. In version of reduced qWarning() calls (print simple line each 100 runs) thread gets executed 3000 times. So I guess it does not depend on amount of qWarning() calls. And why pointer address for SharedData *d is always the same?

main.cpp

int main(int argc, char *argv[])
{
    QApplication a(argc, argv);

    TestThread* thread = new TestThread();

    MainWindow w(thread);
    w.show();

    delete thread;
    return a.exec();
}

mainwindow.h

#ifndef MAINWINDOW_H
#define MAINWINDOW_H

#include <QtGui/QMainWindow>
#include <QThread>
#include <QHash>

class SharedData
{
    public:
        SharedData();
        QString var;
        QHash<QString, QString> hash;
};

class TestThread : public QThread
{
    Q_OBJECT

    public:
        TestThread(QObject *parent = 0);
        void doWork(SharedData* _data);
        void doCrash(QHash<QString, QString>* hash);
    signals:
        void completed(SharedData* d);
    private:
        SharedData* data;
    protected:
        void run();
};

namespace Ui
{
    class MainWindow;
}

class MainWindow : public QMainWindow
{
    Q_OBJECT

public:
    MainWindow(TestThread* t, QWidget *parent = 0);
    ~MainWindow();
    void runThread();
public slots:
    void jobDone(SharedData* req);

private:
    Ui::MainWindow *ui;
    TestThread* t;
    int runcount;
};

#endif // MAINWINDOW_H

mainwindow.cpp

#include "mainwindow.h"
#include "ui_mainwindow.h"
#include <QDebug.h>

TestThread::TestThread(QObject *parent) : QThread(parent)
{
}

void TestThread::run()
{
    qWarning() << "Thread running";
    data->var = "hello";
    doCrash(&data->hash);
    emit completed(data);
}

void TestThread::doWork(SharedData* _data)
{
    data = _data;
    qWarning() << "Attempting to start";
    if(!isRunning())
    {
        run();
    }
    else
    {
        qWarning() << "Oops. Already running";
    }
}

void TestThread::doCrash(QHash<QString, QString>* hash)
{
    hash->insert("test", "123");

    /*
    QHashIterator<QString, QString> i(*hash);
    while (i.hasNext()) {
       i.next();
       qWarning() << i.key() + ":" + i.value();
    }
    */
}

SharedData::SharedData()
{
}

void MainWindow::jobDone(SharedData* req)
{
    qWarning() << "RETURNED";
    qWarning() << "var: " << req->var << " addr: " << &req->var;
    qWarning() << "cnt: " << req->hash.count() << " addr: " << &req->hash;

    QHashIterator<QString, QString> i(req->hash);

    while (i.hasNext()) {
       i.next();
        qWarning() << i.key() + ":" + i.value();
    }

    delete req;
    runThread();
}

MainWindow::MainWindow(TestThread* _t, QWidget *parent)
    : QMainWindow(parent), ui(new Ui::MainWindow)
{
    ui->setupUi(this);

    t = _t;
    connect(t, SIGNAL(completed(SharedData*)), this, SLOT(jobDone(SharedData*)));
    runcount = 0;
    runThread();
}

void MainWindow::runThread()
{
    SharedData* d = new SharedData();
    d->var = "test";

    runcount++;
    qWarning() << "Run count: " << runcount;

    qWarning() << "CREATING THREAD";
    qWarning() << "var: " << d->var << " addr: " << &d->var;
    qWarning() << "cnt: " << d->hash.count() << " addr: " << &d->hash;

    t->doWork(d);
}

MainWindow::~MainWindow()
{
    delete ui;
}
+2  A: 

You shouldn't deleting your TestThread instance in main.cpp (comment out "delete thread;" string) !

vnm
This is correct. You are running things on a deleted thread, which will always lead to undefined behavior. To properly fix things, you should declare an integer to hold the result of a.exec(), then delete the thread, then return the integer.
Caleb Huitt - cjhuitt
Nope, if you remove delete call, same error. I've tried this on second computer.
Pavels
+2  A: 

As vnm has pointed out, the cause of the crash is most likely the delete thread; instruction in main.cpp: when you call w.show() it will return immediately, it is the exec() call that will start the event loop and block, but by then it is too late as the thread has already been deleted.

I would declare the thread as a non-pointer member of the main window instead of passing it as a parameter, this way the compiler will do the cleanup and initialization for you. An even simpler solution would be to use QtConcurrent::run. By doing this you will eliminate all the explicit treading code, but still get the benefits of multi threading.

rpg
Nope, delete was mistake, but it still crashes before delete statement.
Pavels
It's probably a race condition. Try the QtConcurrent approach, it should make your code a lot cleaner and maybe you'll be able to spot the bug.
rpg