QThread run() makes MainWindow get stuck

[kerim]

hi,
i have an application out of which i want to start a thread wich reads a big textfile into a QString but after having started the thread my mainwindow out of which the thread got started gets stuck (means its stuck during thread execution and i dont know why):

here is some code parts:

gui mainwindow (created with designer):

// //////////////////////////////////////////
class MainWindow : public QMainWindow
// //////////////////////////////////////////
{
    Q_OBJECT
 
  // //////////////////////
  public:
  // //////////////////////
  explicit MainWindow(QWidget *parent = 0);
    ~MainWindow();
 
  public slots:
    void AppendTextEdit(QString);
 
  // //////////////////////
  private:
  // //////////////////////
    Ui::MainWindow *ui;
    FileLoader *fileloader;
 
  // //////////////////////
  private slots:
    // //////////////////////
    void on_pushButton_2_clicked();
    void on_pushButton_clicked();
};

// //////////////////////////////////////////
class MainWindow : public QMainWindow
// //////////////////////////////////////////
{
    Q_OBJECT

  // //////////////////////
  public:
  // //////////////////////
  explicit MainWindow(QWidget *parent = 0);
    ~MainWindow();

  public slots:
    void AppendTextEdit(QString);

  // //////////////////////
  private:
  // //////////////////////
    Ui::MainWindow *ui;
    FileLoader *fileloader;

  // //////////////////////
  private slots:
    // //////////////////////
    void on_pushButton_2_clicked();
    void on_pushButton_clicked();
};

To copy to clipboard, switch view to plain text mode 

// //////////////////////////////////
MainWindow::MainWindow(QWidget *parent) :
    QMainWindow(parent),
    ui(new Ui::MainWindow)
// //////////////////////////////////
{
    ui->setupUi(this);
    fileloader = new FileLoader(this);
 
    QObject::connect(fileloader,    SIGNAL(HaveNextChunk(QString)),  this, SLOT(AppendTextEdit(QString)));
 
    ui->textEdit->setCursorWidth(4);
    ui->textEdit->setLineWrapMode(QTextEdit::NoWrap);
 
  }
 
// //////////////////////////////////
MainWindow::~MainWindow()
// //////////////////////////////////
{
    delete ui;
    delete fileloader;
}
 
// //////////////////////////////////
void MainWindow::AppendTextEdit(QString str)
// //////////////////////////////////
{
  QTextCursor tc = ui->textEdit->textCursor();
  tc.movePosition(QTextCursor::End);
  ui->textEdit->setTextCursor(tc);
  ui->textEdit->insertPlainText(str);
}
 
 
// //////////////////////////////////
void MainWindow::on_pushButton_clicked()
// //////////////////////////////////
{
  fileloader->start();
}
 
// //////////////////////////////////
void MainWindow::on_pushButton_2_clicked()
// //////////////////////////////////
{
  fileloader->quit();
}

// //////////////////////////////////
MainWindow::MainWindow(QWidget *parent) :
    QMainWindow(parent),
    ui(new Ui::MainWindow)
// //////////////////////////////////
{
    ui->setupUi(this);
    fileloader = new FileLoader(this);

    QObject::connect(fileloader,    SIGNAL(HaveNextChunk(QString)),  this, SLOT(AppendTextEdit(QString)));

    ui->textEdit->setCursorWidth(4);
    ui->textEdit->setLineWrapMode(QTextEdit::NoWrap);

  }

// //////////////////////////////////
MainWindow::~MainWindow()
// //////////////////////////////////
{
    delete ui;
    delete fileloader;
}

// //////////////////////////////////
void MainWindow::AppendTextEdit(QString str)
// //////////////////////////////////
{
  QTextCursor tc = ui->textEdit->textCursor();
  tc.movePosition(QTextCursor::End);
  ui->textEdit->setTextCursor(tc);
  ui->textEdit->insertPlainText(str);
}


// //////////////////////////////////
void MainWindow::on_pushButton_clicked()
// //////////////////////////////////
{
  fileloader->start();
}

// //////////////////////////////////
void MainWindow::on_pushButton_2_clicked()
// //////////////////////////////////
{
  fileloader->quit();
}

To copy to clipboard, switch view to plain text mode 

now the thread-class

#include <QThread>
 
 
// //////////////////////////////////////////
class FileLoader : public QThread
// //////////////////////////////////////////
{
   Q_OBJECT
 
  // //////////////////////////////
  public:
  // //////////////////////////////
    FileLoader(QObject *parent=NULL);
    ~FileLoader();
    virtual void run();
 
    // //////////////////////////////
   signals:
    //  // //////////////////////////////
      void HaveNextChunk(QString);
 
  // //////////////////////////////
  private:
  // //////////////////////////////
    CLogFile logfile;
    QString chunk;
 
};

#include <QThread>


// //////////////////////////////////////////
class FileLoader : public QThread
// //////////////////////////////////////////
{
   Q_OBJECT

  // //////////////////////////////
  public:
  // //////////////////////////////
    FileLoader(QObject *parent=NULL);
    ~FileLoader();
    virtual void run();

    // //////////////////////////////
   signals:
    //  // //////////////////////////////
      void HaveNextChunk(QString);

  // //////////////////////////////
  private:
  // //////////////////////////////
    CLogFile logfile;
    QString chunk;

};

To copy to clipboard, switch view to plain text mode 

//// //////////////////////////////////
FileLoader::FileLoader(QObject *parent):QThread(parent)
// //////////////////////////////////
{
}
 
//// //////////////////////////////////
FileLoader::~FileLoader()
// //////////////////////////////////
{
}
 
 
// //////////////////////////////////
void FileLoader::run()
// //////////////////////////////////
{
  logfile.Open("big.log");
  if( !logfile.IsOpen() )
    return;
 
//  ulong filesize  = logfile.GetFileSize();
//  ulong bytesRead = 0;
 
  chunk.clear();
  string token;
  while( logfile.GetNextLines(token, 10000) )
  {
    chunk += token.c_str();
  }
  emit HaveNextChunk(chunk);
  logfile.Close();
}

//// //////////////////////////////////
FileLoader::FileLoader(QObject *parent):QThread(parent)
// //////////////////////////////////
{
}

//// //////////////////////////////////
FileLoader::~FileLoader()
// //////////////////////////////////
{
}


// //////////////////////////////////
void FileLoader::run()
// //////////////////////////////////
{
  logfile.Open("big.log");
  if( !logfile.IsOpen() )
    return;

//  ulong filesize  = logfile.GetFileSize();
//  ulong bytesRead = 0;

  chunk.clear();
  string token;
  while( logfile.GetNextLines(token, 10000) )
  {
    chunk += token.c_str();
  }
  emit HaveNextChunk(chunk);
  logfile.Close();
}

To copy to clipboard, switch view to plain text mode 

u can ignore the CLogFile class stuff (it just reads bunch of lines out of textfile and works correct).

the problem is whenever the thread is started out of my mainwindow the whole mainwindow gets stuck until thread has finished work .. why !?!?

thnx


EDIT: ok it seems the insertPlainText(str) call is the bottle-neck --> i guess the string that has to be put into the textedit is quite big (although the file being read is only half-MB of size)

any ideas as alternative approaches to reading file (maybe chunkwise) via thread and updating the textedit as soon as one chunk is read !?!?

[high_flyer]

I wonder who is the first to post the "you are doing it wrong" link...

call is the bottle-neck but dont know why

Because you have a huge text which you read from your huge file, which needs to be transfered to the text edit in the GUI thread.

[wysota]

You should use QTextCursor::insertText() and not QTextEdit::setPlainText().

[kerim]

well ok, thats what i figured out too ,

but it would be very nice if one could give some hint how to do that approach (reading file chunkwise via thread and updating textedit without any bottle necks even if file is huge) !?!?

thnx.

[high_flyer]

You can have a thread reading chunks and adding it to a string, and sending signals to the GUI every time a chunk has been added.
In the GUI slot you append that string to the text edit, and clear the string.
Don't forget to mutex.
While this is done, you might want to have a progress dialog or similar notificatoin to the user that things are in progress.

[wysota]

I don't see why you think he needs a mutex. Maybe I just don't see something but isn't he transfering the string into the main thread through a signal? Simply calling QTextCursor::insertText() instead of QTextEdit::setPlainText() should be enough.

[kerim]

well i have changed the chunk-reading method to:

// //////////////////////////////////
void FileLoader::run()
// //////////////////////////////////
{
  if( !logfile.IsOpen() )
    return;
 
  chunk.clear();
  string token;
  while(logfile.GetNextLines(token, 1000))
  {
    chunk.append(token.c_str());
    emit HaveNextChunk();
  }
}

// //////////////////////////////////
void FileLoader::run()
// //////////////////////////////////
{
  if( !logfile.IsOpen() )
    return;

  chunk.clear();
  string token;
  while(logfile.GetNextLines(token, 1000))
  {
    chunk.append(token.c_str());
    emit HaveNextChunk();
  }
}

To copy to clipboard, switch view to plain text mode 

the appropriate slot within my maingui is named

// //////////////////////////////////
void MainWindow::AppendTextEdit()
// //////////////////////////////////
{
  QString str = fileloader->Chunk();
  QTextCursor tc = ui->textEdit->textCursor();
  tc.movePosition(QTextCursor::End);
  tc.insertText(str);
  ui->textEdit->setTextCursor(tc);
  //ui->textEdit->insertPlainText(str);
}

// //////////////////////////////////
void MainWindow::AppendTextEdit()
// //////////////////////////////////
{
  QString str = fileloader->Chunk();
  QTextCursor tc = ui->textEdit->textCursor();
  tc.movePosition(QTextCursor::End);
  tc.insertText(str);
  ui->textEdit->setTextCursor(tc);
  //ui->textEdit->insertPlainText(str);
}

To copy to clipboard, switch view to plain text mode 

as u can see i have even changed the inserting method as proposed from one forum-reader.
when i set a breakpoint during debugging i can see that reading the chunks is quite fast but inserting the text is slow.

details: the

fileloader->Chunk();

fileloader->Chunk();

To copy to clipboard, switch view to plain text mode 

call returns a reference to a string (the current chunk being read). since the code has no mutex mechanism i dont know if this could be the problem: while trying to insert string into textedit the chunk-str is allready updated via file-reading thread and everything gets messed up .. i will try to insert some locking mechanism and give u feedback then.

otherwise: if somebody thinks the reason is something different: post it

[high_flyer]

@wysota:
The problem I see with transfering QStrings over signals is, that if the slot is slower, potentially large number of signals queue can accumulate.
And it looks my fear was justified:

when i set a breakpoint during debugging i can see that reading the chunks is quite fast but inserting the text is slow.

I find it more elegant, if the thread just appends to one QString, which is available to the GUI and which the GUI clears every time it got it, and in that access method, there is a need to mutext that string.
Something like:

//In the thread:
void MyThread::run()
{
...
m_mutex.lock();
m_strData += getChunkFromFile();
Q_EMIT sig_newData();
m_mutex.unlock();
...
}
 
QString MyThread::GetStringData()
{
  QMutxLocker(&m_mutex);
   QString strTemp = m_strData;
   m_strData.clear();
   return strTemp;
}
 
//And in the GUI
void GuiClass::onNewData()
{
  ui.txtEdit->insertText(m_pThread->GetStringData());
}

//In the thread:
void MyThread::run()
{
...
m_mutex.lock();
m_strData += getChunkFromFile();
Q_EMIT sig_newData();
m_mutex.unlock();
...
}

QString MyThread::GetStringData()
{
  QMutxLocker(&m_mutex);
   QString strTemp = m_strData;
   m_strData.clear();
   return strTemp;
}

//And in the GUI
void GuiClass::onNewData()
{
  ui.txtEdit->insertText(m_pThread->GetStringData());
}

To copy to clipboard, switch view to plain text mode 

[Lesiok]

FileLoader::run() method is locking event dispatcher. Look at example int QThread doc.

[wysota]

The only thing that needs changing in my opinion is that the signal needs to carry the text so that the slot doesn't have to fetch it from the other object as this indeed requires a mutex.

class FileReader : public QObject {
  Q_OBJECT
public:
  FileReader(){}
public slots:
  void readFile(const QString &filePath) {
    QFile f(filePath);
    if(!f.open(QFile::ReadOnly|QFile::Text)) return;
    while(!f.atEnd()) emit nextLine(f.readLine());
  }
signals:
  void nextLine(QString);
};
 
class Receiver : public... {
  Q_OBJECT
public slots:
  void appendLineToTextEdit(const QString &line) {
    QTextCursor cursor(textEdit->document());
    cursor.movePosition(QTextCursor::End);
    cursor.insertText(line);
  }
};
// ...
QThread thread;
thread.start();
FileReader reader;
connect(&reader, SIGNAL(nextLine(QString)), this, SLOT(appendLineToTextEdit(QString)));
reader.moveToThread(&thread);
QMetaObject::invokeMethod(&reader, "readFile", "somefile.txt", Qt::QueuedConnection);

class FileReader : public QObject {
  Q_OBJECT
public:
  FileReader(){}
public slots:
  void readFile(const QString &filePath) {
    QFile f(filePath);
    if(!f.open(QFile::ReadOnly|QFile::Text)) return;
    while(!f.atEnd()) emit nextLine(f.readLine());
  }
signals:
  void nextLine(QString);
};

class Receiver : public... {
  Q_OBJECT
public slots:
  void appendLineToTextEdit(const QString &line) {
    QTextCursor cursor(textEdit->document());
    cursor.movePosition(QTextCursor::End);
    cursor.insertText(line);
  }
};
// ...
QThread thread;
thread.start();
FileReader reader;
connect(&reader, SIGNAL(nextLine(QString)), this, SLOT(appendLineToTextEdit(QString)));
reader.moveToThread(&thread);
QMetaObject::invokeMethod(&reader, "readFile", "somefile.txt", Qt::QueuedConnection);

To copy to clipboard, switch view to plain text mode 

[kerim]

FileLoader::run() method is locking event dispatcher. Look at example int QThread doc.

as i have written previously:
i dont think its the thread or its run method being the reason for the stuck issue:

to me (as far as i understand with my humble experience in regard to Qt) the problem occurs whenever we have a situation where run (reading lines from file) is so fast that emiting the signal HaveNextChunk() is done faster and more than inserting the read lines into the text-edit

@wysota:
The problem I see with transfering QStrings over signals is, that if the slot is slower, potentially large number of signals queue can accumulate.
And it looks my fear was justified:

I find it more elegant, if the thread just appends to one QString, which is available to the GUI and which the GUI clears every time it got it, and in that access method, there is a need to mutext that string.
Something like:

//In the thread:
void MyThread::run()
{
...
m_mutex.lock();
m_strData += getChunkFromFile();
Q_EMIT sig_newData();
m_mutex.unlock();
...
}
 
QString MyThread::GetStringData()
{
  QMutxLocker(&m_mutex);
   QString strTemp = m_strData;
   m_strData.clear();
   return strTemp;
}
 
//And in the GUI
void GuiClass::onNewData()
{
  ui.txtEdit->insertText(m_pThread->GetStringData());
}

//In the thread:
void MyThread::run()
{
...
m_mutex.lock();
m_strData += getChunkFromFile();
Q_EMIT sig_newData();
m_mutex.unlock();
...
}

QString MyThread::GetStringData()
{
  QMutxLocker(&m_mutex);
   QString strTemp = m_strData;
   m_strData.clear();
   return strTemp;
}

//And in the GUI
void GuiClass::onNewData()
{
  ui.txtEdit->insertText(m_pThread->GetStringData());
}

To copy to clipboard, switch view to plain text mode 

i tried ur proposal (the line where QMutexlocker is created should look like this:

QMutexlocker locker(&mutex);

QMutexlocker locker(&mutex);

To copy to clipboard, switch view to plain text mode 

otherwise compiler complains with unintialized reference mutex (!!? dont know why).
anyway:
this is what came out:

the application is still slow and the textedit does not update immediately: while debugging i saw that emitting the HaveNextChunk() signal is emitted several times without landing on the slot break-point (the slot is not executed until whole file is read and gets executed afterwards as much as times as there where emits previously) -> thats not what i intented to do

anybody got some hints (documentation reference where i can read about this problem, or other code proposals) !?!?!

thnx alot

---

Added after 6 minutes:

FileLoader::run() method is locking event dispatcher. Look at example int QThread doc.

where exactly is the example ur talking about !?!? i would like to read it.

i looked at the installed QT example under the threads folder named "queuedcustomtype" and it seems this example is nearly exactly constructed as mine. but when i debug it i can see that after each emit the appropriate slot is handled while my application does not, but i can not figure out why

[high_flyer]

i tried ur proposal (the line where QMutexlocker is created should look like this:

My code is pseudo code, which I typed here on the forum, so such mistakes are very likely, sorry.
The complier is correct, a variable needs to be defined as well, not only the type.
But the code is only to convey the idea, its not meant to be copy paste working solution.

the application is still slow

You can't avoid this, since you have a heavy operation on a hardware device, you will have to live with it.
The only thing you can do, is make your GUI fully responsive while this is going on, and as I said, add a progress bar or similar means of letting the user know that this operation will take time.

The solution offered here, both wasota's flavor and mine, are not to speed the application, but to allow fully responsive GUI while the data is being loaded and updated.
If the GUI is still not fully responsive, then there is still a problem with the implementation (maybe the chunk data size is still too large, try limiting it to 100 bytes per read or so.)

while debugging i saw that emitting the HaveNextChunk() signal is emitted several times without landing on the slot break-point

That is to be expected - since signals between threads are queued - and that is the reason I didn't go for sending strings via the signals, which in this case are being fired very quickly, and many of them.

anybody got some hints (documentation reference where i can read about this problem, or other code proposals) !?!?!

Read about queued connections in the documentation:
http://doc.trolltech.com/main-snapsh...s-qobject.html

[wysota]

the application is still slow and the textedit does not update immediately:

If you want fast then either do the reading in the main thread and live with the fact that your app will be unresponsive for that time or create the text document in a worker thread and push it to the main thread when it is ready and then set it on the text edit. Also use QPlainTextEdit instead of QTextEdit if you're not operating on rich-text. Any other interleaving between the main and the worker thread will make your application slower.

[kerim]

well after having read quite a bunch of sites about the threading theme within qt i am more confused then before, especially what i have read in this article:
http://labs.qt.nokia.com/2010/06/17/...oing-it-wrong/

.. and now i know what high_flyer ment by mentioning the doing-it-wrong stuff.

at this point i dont have a clue how to get things done via QThread, my main problem is still
that my gui gets frozen when the thread is running and i realy would like to know why and how
to solve this.
if necessary i can post my whole src-code although the critical parts are allready posted.

as i mentioned, i also looked up how the example Qt-projects in regard to threads are constructed, in specific the "queuedcostumtype" example and its constructed the same way as my application
except that small rectangular images are sent to the main-thread (which in turn paints them) rather than reading lines from textfile and sending strings.
the example seems not to get stuck (main-windows is movable) but mine does, so why !?!?!?!

please help.

---

Added after 6 minutes:

If you want fast then either do the reading in the main thread and live with the fact that your app will be unresponsive for that time or create the text document in a worker thread and push it to the main thread when it is ready and then set it on the text edit

that surprises me alot because i am sure that a qt framework should be able to get things managed in a more suitable satisfactory way and i am quite sure that it is possible. the only question is:
how complex to achive that ?

and i really dont know the answer ? there must be a simple example as a solution for this.

[high_flyer]

that surprises me alot because i am sure that a qt framework should be able to get things managed in a more suitable satisfactory way and i am quite sure that it is possible.

Look, as stated before, if you have large amount of data it takes time.
Its always the same - speed and resources are interchangeable.
If you want more speed, you will have to work with less resources, or, if you want to work with large amount of resources, you will get a speed penalty.
So Qt (or any other software solution for that matter) can't eliminate your need to read a large file and show it, so the time it will take to read the file, and show the data is time you can only shorten via better hardware.

the example seems not to get stuck (main-windows is movable) but mine does, so why !?!?!?!

If you are using wysota's solution, my guess is that it has to do with the amount of data queued through the signals sent from the worker thread.
If you are using mine, then as I said, it has probably to do with a too large chunck size - try limiting it.

Otherwise, you are doing something else wrong (as maybe not REALLY using a thread even if you think you are, due to thread affinity problem) or similar, that we can't see with the information you provided so far.

[wysota]

that surprises me alot because i am sure that a qt framework should be able to get things managed in a more suitable satisfactory way and i am quite sure that it is possible.

It's not about Qt. It's about how computers work. If you have more then one thread then the total amount of work that the CPU must do is larger than with a single thread because of context switching and synchronisation. If you don't have enough CPU power (i.e. the number of cores or chips) the difference might be significant. Since you are using a lot of signals across threads that are synchronized, you waste a lot of cycles on the synchronization. This starts an avalanche effect - you send new data to the document, the widget needs to refresh itself, the document is reformatted, the widget is redrawn but there is already more data waiting to be handled and the whole operation starts again. The main thread is busy handling the data flowing in.

the only question is:
how complex to achive that ?

5-6 lines of code if you want it dirty, a couple more if you want a clean solution.

And you can happily do it in one thread, the loading will just take longer as you need to save some cpu time for the user.

[Lesiok]

---

Added after 6 minutes:

where exactly is the example ur talking about !?!? i would like to read it.

i looked at the installed QT example under the threads folder named "queuedcustomtype" and it seems this example is nearly exactly constructed as mine. but when i debug it i can see that after each emit the appropriate slot is handled while my application does not, but i can not figure out why

You don't start event loop in FileLoader. Standard QThread::run implementation in last step callis QThread::exec. It is necessary to call this function to start event handling. (from QThread::exec doc).

[kerim]

allright, this is how i am going to do it:

i subclassed QThread and load content of the file into buffer (wich is not emediately shown within textedit in mainwindow). that makes the whole loading stuff quite fast and the main-gui is still usable while thread is running.
this is much more efficient in regard to gui-being stuck because the signal emitted now is just telling the main-gui to updated its progressbar.

(only after whole loading process has finished there is one signal emitted to the main-thread wich is for signaling that loading has finished). this takes me almost 2seconds for a 250MB textfile which is ok.

now, i am going to save the whole content within a buffer and try to set it into the text-widget after the loading has finished.

now comes my question: how do i do that most efficiently ??
the more the size of the QString being appended to QTextEdit is, the slower things are getting.
1. so using some Timer which puts every 1000 lines per second until for the whole data-buffer could be one option.
but for a 250MB file this could take time.

2. another option would be to write some class wich manages when to read data from buffer and put it to textedit (only when user scrolls down beyond are being viewed)

some (maybe better) ideas about that ?

thnx anyway.

[wysota]

I have already suggested a solution that does what you want with the only exception that the text document is transfered instead of raw text. This should make the whole process much much faster.

[kerim]

I have already suggested a solution that does what you want with the only exception that the text document is transfered instead of raw text. This should make the whole process much much faster.

thnx wysota for ur suggestion.
but isnt it bad that one thread manipulates gui belonging to other thread ?

how would/could code look (just small example) that uses local QTextDocument which is transfered and used from gui after worker-thread has finished??

i really appreciate all the efforts of people responding here and i am very thankfull .
hope the next reply will be the last )

thnx.