PDA

View Full Version : QThread run() makes MainWindow get stuck



kerim
24th March 2011, 13:31
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();
};




// //////////////////////////////////
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();
}



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;

};




//// //////////////////////////////////
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();
}


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
24th March 2011, 14:15
I wonder who is the first to post the "you are doing it wrong" link... :rolleyes:


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
24th March 2011, 14:19
You should use QTextCursor::insertText() and not QTextEdit::setPlainText().

kerim
24th March 2011, 14:20
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
24th March 2011, 14:25
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
24th March 2011, 14:32
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
24th March 2011, 14:41
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();
}
}


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);
}

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();

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
24th March 2011, 14:44
@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());
}

Lesiok
24th March 2011, 15:21
FileLoader::run() method is locking event dispatcher. Look at example int QThread doc.

wysota
24th March 2011, 15:27
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);

kerim
25th March 2011, 09:28
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());
}



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

QMutexlocker locker(&mutex); 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
25th March 2011, 10:59
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-snapshot/threads-qobject.html

wysota
25th March 2011, 11:18
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
25th March 2011, 11:59
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/youre-doing-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.:confused:

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
25th March 2011, 12:14
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
25th March 2011, 15:18
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
25th March 2011, 17:32
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
25th March 2011, 17:36
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
25th March 2011, 17:40
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
26th March 2011, 13:20
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.

wysota
27th March 2011, 01:10
but isnt it bad that one thread manipulates gui belonging to other thread ?
QObjects "belong" to the thread that creates them until you push them to a different thread using moveToThread(). So if you create the object in the worker thread and initialize it with data there and then push the result to the main thread, it is likely it will work.


how would/could code look (just small example) that uses local QTextDocument which is transfered and used from gui after worker-thread has finished??
Sorry, I'm done with doing one's work for him. You have been given all needed details. Dig in and have fun.

kerim
27th March 2011, 13:57
@wysota:
thnx alot man, the first part of ur last post was the info i needed and in regard to the second part: i didnt wanted you to come up with a hundrets-of-lines example :), the point that made me suspicous about manipulating gui beyond threads was just made clear by the first part of ur comment ;), thnx alot.

i ll try that.
cheers.

Added after 59 minutes:

:confused: i am still missing something i think:

my current relevant code parts in regard to the worker-thread declaration are:

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

// //////////////////////////////
public:
// //////////////////////////////
FileLoader(QObject *parent);
~FileLoader();

signals:
void WorkDone(QTextDocument *);

protected:
void run();

// //////////////////////////////
private:
// //////////////////////////////
CLogFile logfile;
QTextDocument *doc;
};

(everytime before workerthread manipulates QTextDocument member its calling moveToThread(this) within its running method):


// //////////////////////////////////
void FileLoader::run()
// //////////////////////////////////
{
...
string content;

doc->moveToThread(this);
QTextCursor cursor(doc);

// manipulate doc here ...

emit WorkDone(doc);

}


the workerthread emits once the WorkDone(QTextDocument *) signal when work is done wich is caught by the main-threads slot

void PutDoc(QTextDocument *doc)
{
doc->moveToThread(QApplication::instance()->thread());
ui->textEdit->setDocument(doc);
}

the last line here (setDocument) fails and throws a segmentation fault which crashes my application, what am i doing wrong now !?!?
here is the trace:
http://img132.imageshack.us/f/11178576.gif/

wysota
27th March 2011, 14:59
It won't work this way. You are trying to "pull" an object into your own thread instead of "pushing" it out of the thread it lives in. This probably fails and you get a segfault when trying to access an object that doesn't belong to you.

kerim
27th March 2011, 17:42
It won't work this way. You are trying to "pull" an object into your own thread instead of "pushing" it out of the thread it lives in. This probably fails and you get a segfault when trying to access an object that doesn't belong to you.

ok, so whats the difference between "pushing" and "pulling" the object ?
i just tried it with the moveToThread(..) thing u mentioned.

if this is some general issue problem or something about to be read from some documentation, i appreciate read links too.

wysota
27th March 2011, 17:46
You "pull" things that are far away towards you, you "push" things that are close to you away. So in our situation it means you can move the object from your thread to some other thread (push) but not from another thread to your own thread (pull) - you can't "steal" an object from another thread. It's all in QObject::moveToThread() docs - see the warning at the end.

kerim
27th March 2011, 22:32
You "pull" things that are far away towards you, you "push" things that are close to you away. So in our situation it means you can move the object from your thread to some other thread (push) but not from another thread to your own thread (pull) - you can't "steal" an object from another thread. It's all in QObject::moveToThread() docs - see the warning at the end.


well it sais:
"Warning: This function is not thread-safe; the current thread must be same as the current thread affinity. In other words, this function can only "push" an object from the current thread to another thread, it cannot "pull" an object from any arbitrary thread to the current thread."

i assume with saying current thread the thread calling moveToThread is ment, but what is thread-affinity in this context (does that mean the adress space of the thread)!?!?

i really dont have a clue how to achive a none crashing example:
- where should the QTextDocument be created (heap or stack ?)
- how do i pass the QTextDocument to the main-thread to set it for the QTextEdit?
:(

wysota
27th March 2011, 23:24
I really wouldn't like to repeat what is already Qt docs. Read Threads and QObjects