CODEWITHSUNDEEP
c++multithreadingqtc++11
Ufx
Ufx

Reputation: 2695

Qt GUI doesn't work with std::thread as I expect

The core of my project is independent of GUI framework that's why I prefer std::thread. But Qt gives me an error when thread is using.

The inferior stopped because it received a signal from the operating system.

Signal name: SIGSEGV
Signal meaning: Segmentation fault

//MainWindow.h
#ifndef MAINWINDOW_H
#define MAINWINDOW_H

#include <thread>
#include <mutex>
#include <QMainWindow>

namespace Ui { class MainWindow; }

struct Observer
{
    virtual void notify() = 0;
};

class Core
{
public:
    std::thread *run()
        {
            std::thread thread(&Core::runP, this);
            thread.detach();
            return &thread;
        }

    void setObserver(Observer *observer) { _observer = observer; }
    int ii() const { return _ii; }
    void nextIi() { _ii++; }

    void lock()    { _mutex.lock(); }
    bool tryLock() { return _mutex.try_lock(); }
    void unlock()  { _mutex.unlock(); }

private:
    void runP()
        {
            for (int i = 1; i <= 1000; i++) {
                if (i % 10 == 0) {
                    lock();
                    nextIi();
                    unlock();
                    notify();
                }
            }
        }

    void notify() { _observer->notify(); }  //!!!
    Observer *_observer;
    int _ii;
    std::mutex _mutex;
};

struct MwObserver : public Observer
{
    explicit MwObserver(struct MainWindow *mainWindow) { _mainWindow = mainWindow; }
    virtual void notify();

    MainWindow *_mainWindow;
};

class MainWindow : public QMainWindow
{
    Q_OBJECT

public:
    explicit MainWindow(QWidget *parent = 0);
    ~MainWindow() { delete _ui; }
    void upd();

public slots:
    void run() { _core.run(); }

private:
    Ui::MainWindow *_ui;
    MwObserver _observer;
    Core _core;
};

inline void MwObserver::notify() { _mainWindow->upd(); }

#endif

-

//MainWindow.cpp
#include "mainwindow.h"
#include "ui_mainwindow.h"

MainWindow::MainWindow(QWidget *parent) :
    QMainWindow(parent),
    _ui(new Ui::MainWindow),
    _observer(this)
{
    _ui->setupUi(this);
    connect(_ui->pushButtonRun, SIGNAL(clicked(bool)), this, SLOT(run()));
}

void MainWindow::upd()
{
    _core.lock();
    setWindowTitle(QString::number(_core.ii()));
    _core.unlock();
}

Upvotes: 6

Views: 3321

Answers (7)

Thomas
Thomas

Reputation: 5138

The answers to all the problems have already been given, let me summarize.

The program crash has nothing to do with the threading, The problem is that the _observer in the _core member of MainWindowis not set. A call to setObserver must be added.

explicit MainWindow( QWidget *parent = nullptr ) :
    QMainWindow( parent ),
    _observer( this )
{
    _core.setObserver( &_observer );
}

This will lead to the next problem, that the observer actually calls the udp message from another thread, causing a UI update in a different thread context. To solve this, it is easiest to use Qt's Qt::QueuedConnection. To enable this we must make upt() a slot.

public slots:

void run();
void upd();

Then we can either call it using QMetaObject::invokeMethod in 

inline void MwObserver::notify()
{
    QMetaObject::invokeMethod( _mainWindow, "upd", Qt::QueuedConnection );
}

or use a signal / slot connection by deriving MwObserver from QObject, giving it a signal, and connect that signal to the upd slot and raising the signal in notify. 

struct MwObserver 
    : public QObject
    , public Observer
{
    Q_OBJECT;

    signals:
    void sigUpd();

public:
    explicit MwObserver( MainWindow *mainWindow );

    virtual void notify()

    MainWindow *_mainWindow;
};

void MwObserver::notify()
{
    sigUpd();
}

MwObserver::MwObserver( MainWindow *mainWindow )
{
    _mainWindow = mainWindow;
    connect( this, SIGNAL(sigUpd()), _mainWindow, SLOT(upd()) )
}

Upvotes: 2

Elohim Meth
Elohim Meth

Reputation: 1827

Aside from returning pointer to stack variable and updating GUI from thread object that is not known for QT. I don't see from your code, where you set up _observer member of Core class. There is no setObserver call for _core member of MainWindow class.

So consructor of MainWindow class calls consructor of _core member, but after that _core._observer contains garbage. I think this is the cause of your Segmentaion Fault in call of notify method of Core class.

Upvotes: 3

Marek R
Marek R

Reputation: 37512

The best approach is to wrap pure C++ code with QObejct instance and fire signals when this objects receive some notification from pure C++ code.

SO in your case:

class MwObserver : public QObject, public Observer
{
    Q_OBJECT
public:
    explicit MwObserver(QObject *parent)
      : QObject(parent)
    {}

signals:
    void SomeEvent();

protected:
    // Observer
    void notify() {
       emit SomeEvent();
    }
};

Now MainWindow should connect some slot to signal provided this way and everything should work out of the box (Qt will do thread jumping behind the scenes).

In your code form comment the crash is caused by invalid use of temporary object. This is INVALID C++ code no mater what kind of object is returned:

std::thread *run()
{
    std::thread thread(&Core::runP, this);
    thread.detach();
    return &thread;
 }

You cant return a pointer to local object of the function method, since this object becomes invalid immediately when you return a function. This is basic C++ knowledge.

Upvotes: 0

Thaddeus
Thaddeus

Reputation: 84

Disclaimer: I haven't used Qt in some time but with X/XMotif on Linux/UNIX the GUI MUST run in the 'main-thread', not spawned threads. Maybe this applies to your situation. Just a thought, have your GUI code run in the main-thread.

Upvotes: 0

UldisK
UldisK

Reputation: 1639

There are multiple problems here, first and most obvious was already noted by perencia. You are returning a pointer to stack variable. In c++ terms it's unacceptable.

Secondly. The crash comes from not using std::thread, but from race condition. The Qt event loop does not know about you mutex, so your setWindowTitle call is introducing a race, that leads to crash. You need to use QMetaObject::invokeMethod to post function to the Qts event loop.

Example: change

inline void MwObserver::notify() { _mainWindow->upd(); }

to 

inline void MwObserver::notify() {
    if(!QMetaObject::invokeMethod(_mainWindow, "upd", Qt::QueuedConnection))
        std::cerr << " Failed to invoke method" << std::endl;
}

additional includes may apply

Upvotes: 11

jonjonas68
jonjonas68

Reputation: 625

This updates the GUI from a thread different then the GUI thread! Which is not allowed. Why not to use QThread and a signal/slot mechanism to update your window title. The Qt framework does the thread switching automatically.

class Core : public QObject 
{
  Q_OBJECT
public:
  explicit Core(QObject * parent = 0) : QObject(parent) {}

signals:
  void notify();

public slots:
  void nextIi() { _ii++; }
  void runP()
  {
    for (int i = 1; i <= 1000; i++) {
      if (i % 10 == 0) {
        nextIi();
        notify();
      }
    }
  }

private:
  Q_DISABLE_COPY(Core);
  int _ii;
};

class MainWindow : public QMainWindow
{
    Q_OBJECT 
public:
    explicit MainWindow(QWidget *parent = 0);
    ~MainWindow();

public slots:
    void run() {_th.start();}
    void upd(int ii) {setWindowTitle(QString::number(ii));}

private:
    Ui::MainWindow *_ui;
    Core _core;
    QThread _th;
};

//MainWindow.cpp
#include "mainwindow.h"
#include "ui_mainwindow.h"

MainWindow::MainWindow(QWidget *parent) :
    QMainWindow(parent),
    _ui(new Ui::MainWindow),
    _observer(this)
{
  _ui->setupUi(this);

  connect(_ui->pushButtonRun, SIGNAL(clicked(bool)), this, SLOT(run()));
  connect(&_core, SIGNAL(notify(int)), this, SLOT(upd(int)));
  _core.moveToThread(&_th);
}

MainWindow::~MainWindow()
{ 
  delete _ui; 
  _th.quit();
  _th.wait(1000);
}

Upvotes: 9

perencia
perencia

Reputation: 1542

You are creating thread on the stack and returning a pointer to that. After run() that pointer is no longer valid.

Upvotes: 7

Related Questions