PDA

View Full Version : QThread exec proplem to stop...



patrik08
20th May 2007, 13:32
this is my 2° QThread wo i write ...
the class to get remote dir list work on apache2 now to get recursive dir and file list i like a QThread to speed up all... how i can tell QThread to paint data to a model...

if i make a
connect(this, SIGNAL(finished()), this, SLOT(PaintData()));
not work ... but data exist...

all class are on main attachment...




class DavListDir : public QThread
{
Q_OBJECT

public:
void run();
void SetUrl( QUrl u );
signals:
void statusMessage(QString);
void TimeEnd();
private:
QUrl urltogrep;
HTTP_propfind *jobb;
public slots:
void PaintData();
};
void DavListDir::SetUrl( QUrl u )
{
urltogrep = QUrl(u);
connect(this, SIGNAL(finished()), this, SLOT(PaintData()));
}

void DavListDir::PaintData()
{
/* prepare to paint remote dir list on tree model */
std::cout << "end cycle " << std::endl;
std::cout << qPrintable(jobb->GetDAVData()) << std::endl;
}

void DavListDir::run()
{
jobb = new HTTP_propfind(urltogrep);
jobb->Start();
exec();
}

marcel
20th May 2007, 15:26
As I told you some time ago, you cannot update widgets from any other thread than the GUI thread.

There are a few solutions to this, but all involve passing the model data to the GUI thread.
In your case this is fairly simple. Since the data is complete (OK) when finished(), then connect the finished() signal somewhere in your GUI thread.

Then, when it is emitted, in the slot from the GUI thread just get thread->job->GetDAVData() before you delete the thread.

I hope you understand what I mean.

Another solution is to register the type of jobb as a Qt metatype and pass it through a signal to the GUI thread.

But the first solution is simpler and will work for you.

Regards

patrik08
20th May 2007, 16:25
Yes but on cast can i not access the objekt QThread ?

DavListDir *incomming = qobject_cast<DavListDir *>(sender());

QThread not emit finished ?? why?

QThread is hard eat..

and so poor examples .. on the new code search... google.

http://www.google.com/codesearch?hl=it&lr=&q=QThread+QThread+%2Bfinished%28%29+%2Bexec+lang%3 Ac%2B%2B







/* ############################# QThread start method ###*/

class DavListDir : public QThread
{
Q_OBJECT

public:
void run();
QString GetxmlData();
void SetUrl( QUrl u );
signals:
void statusMessage(QString);
void TimeEnd();
private:
QUrl urltogrep;
HTTP_propfind *jobb;
public slots:
};
void DavListDir::SetUrl( QUrl u )
{
urltogrep = QUrl(u);
}
QString DavListDir::GetxmlData()
{
return jobb->GetDAVData();
}

void DavListDir::run()
{
jobb = new HTTP_propfind(urltogrep);
jobb->Start();
exec();
}


/* ############################# compose dir tree #*/


class Tree_Dir_LS : public QObject
{
Q_OBJECT
//
public: Tree_Dir_LS( QObject* = 0 )
{
passage = 0;
std::cout << "construct class " << std::endl;
ls_a(QUrl("http://user:pass@192.168.1.33/webdav/"));
ls_a(QUrl("http://user:pass@192.168.1.33/webdav/www/"));
ls_a(QUrl("http://user:pass@192.168.1.33/webdav/www/notexist/"));

}

public:
void ls_a( QUrl dir )
{
DavListDir *cann = new DavListDir();
cann->SetUrl(dir);
cann->start(QThread::HighPriority);
connect(cann, SIGNAL(finished()), this, SLOT(PaintData()));
}

protected:
int passage;
private:
signals:
public slots:
void PaintData()
{
passage++;
DavListDir *incomming = qobject_cast<DavListDir *>(sender()); /* any sender dir level having any info from dir... path e file */
std::cout << "----------------append dir tree nr. " << passage << " --------------" << std::endl;
std::cout << qPrintable(incomming->GetxmlData()) << std::endl;
incomming->quit();
}

};

patrik08
20th May 2007, 17:03
Is this mistake if i say QObject can not get QThread finisch signal; only Gui like QWidget, QDialog, QMainWindow can du that?

This QThread must build recursive remote dir and get info to pass on QObject... like a dirmodel.

marcel
20th May 2007, 17:14
No, they should get the finished signal.
The only thing you must worry about is the thread finish condition.
I mean, when does the thread stop? Are the recursion conditions correct?
If you can get these straight, then the thread will exit OK and you will get the finished signal, therefore be able to extract the data.

Please correct me if I'm wrong.

Regards

patrik08
20th May 2007, 17:51
No, they should get the finished signal.
The only thing you must worry about is the thread finish condition.
I mean, when does the thread stop? Are the recursion conditions correct?
If you can get these straight, then the thread will exit OK and you will get the finished signal, therefore be able to extract the data.

Please correct me if I'm wrong.
Regards

yes this is correct ... on the QObject i call the QThread wo self exec() this call
HTTP_propfind( QUrl urigo ) <- this emit only a return QString xml ls info from dir or empty..
on error...

on subclass QHttp i have only two signal..

connect(this, SIGNAL(requestFinished(int, bool)), this, SLOT(EndTakeData(int, bool)));
connect(this, SIGNAL(done(bool)), this, SLOT(abort()));

can this break QThread?

can this break finished() signal ... i show on main.moc and i found the signal from QThread -> QObject.....



void ls_a( QUrl dir )
{
DavListDir *cann = new DavListDir(); /* QThread */
cann->SetUrl(dir);
cann->start(QThread::HighPriority);
connect(cann, SIGNAL(finished()), this, SLOT(Read_LS_all()));
}

void Read_LS_all()
{
DavListDir *incomming = qobject_cast<DavListDir *>(sender());
QString xmlls = incomming->GetxmlData();
append xmlls to tree..
..........


or must i start the QThread on QObject
class....
cann->start(QThread::HighPriority); ++
cann->wait() ++

like $QTDIR/examples/threads/waitconditions ?

note at end on main.cpp i have

#include "main.moc"

can this break signal.. ? i think no if i leave QThread the ls dir QHttp runn corecet..

marcel
20th May 2007, 18:01
Creating the QHttp object does not break anything because you start the thread's event loop with exec. QHttp will emit timer events from time to time, so it needs an event loop.

How were you starting the thread until now?
The correct way is calling start ( you can do that from the thread's creator/parent ).
Once you call start, the run() method is executed. After it leaves run() the finished signal will be emitted.
So if you don't get the finished signal it means that something goes wrong in run().

About wait conditions: why do you use them? Do you have any shared data that the GUI thread and the worker thread have to read/write?
From what I have seen you don't need wait conditions. Not even a mutex, because, from what you said, the complete data( the directory structure ) is available only when the thread finishes.

You don't need any casts, or anything. Just in the thread's creator, when you receive the finished signal, get the data and update the model.

Could you post the code section where you create the thread and the whole current version of the thread code?
And please do not post any additional code, because it tends to be misleading :).

Regards

patrik08
20th May 2007, 18:10
Creating the QHttp object does not break anything because you start the thread's event loop with exec. QHttp will emit timer events from time to time, so it needs an event loop.

How were you starting the thread until now?
The correct way is calling start ( you can do that from the thread's creator/parent ).
Once you call start, the run() method is executed. After it leaves run() the finished signal will be emitted.
So if you don't get the finished signal it means that something goes wrong in run().
Could you post the code section where you create the thread and the whole current version of the thread code?
And please do not post any additional code, because it tends to be misleading :).

Regards

on this case the mistake is that all run on main.cpp if QObject send and take finisch

all code is on one main.cpp attachment...

pro file ...



TEMPLATE = app
TARGET =
DEPENDPATH += .
INCLUDEPATH += .
QT += xml
QT += network
CONFIG += console
CONFIG += qt warn_off release
LANGUAGE = C++
TARGET = xx
DESTDIR = ./
SOURCES += main.cpp

marcel
20th May 2007, 18:33
OK, just took a look on your code.
jobb->Start will exit immediately.

What I have missed to tell you is that when using QThread::exec, to stop the thread you either must destroy the main widget or call QThread::exit() or QThread::quit().

You must do one of the last two.
You must call them when QHttp has finished. This means that you will have to add a signal in HTTP_propfind that will be connected to a slot in the thread ( endThread for example ).

You must emit the signal from HTTP_propfind when QHttp has finished all request ( meaning when you are sure everything is done ).
In the endThread slot you just call exit(0) or quit().
This sequence results in the finished signal getting emitted.

However - I noticed you use the thread only for the QHttp requests. QHttp alone is not designed(thought) to be used in a separate thread.
It is asynchronous and state driven - meaning it will not block the parent thread wile working.
On the other hand, there is nothing wrong in using QHttp like you did - just some extra code.
Anyway, it is your decision.

Regards

wysota
20th May 2007, 18:44
The correct way is calling start ( you can do that from the thread's creator/parent ).
Once you call start, the run() method is executed. After it leaves run() the finished signal will be emitted.
Actually, it's a little different if you want to use signals within the thread. See my next comment.


So if you don't get the finished signal it means that something goes wrong in run().


One has to call exec() within run() to start the event loop for the thread so that signals can be emitted. Without it QHttp or any other class that uses events (signals included) won't function properly. Please note that this doesn't concern the finished() signal as it is emitted from the parent (where QThread object was created) thread.

marcel
20th May 2007, 18:59
Yes, exactly what I said.
But Patrik's problem is that he cannot stop the thread :).
Everything works well ( he starts the event loop and all ), but the thread doesn't ever finish because he wasn't calling exit or quit anywhere.



One has to call exec() within run() to start the event loop for the thread so that signals can be emitted.
Not so sure about that. Signals can be emitted from within the thread to other threads - this works because the are nothing but some events. It is similar to calling QApplication: postEvent( someThread, someEvent ).



Without it QHttp or any other class that uses events (signals included) won't function properly
Yes, any event that targets this thread( without an event loop ) is a potential danger to the entire application ( therefore to other threads ).
It happened to me - I was trying to create a pixmap( load it from disk ) in some thread that didn't have an event loop. But QPixmapCache( used internally by QPixmap ) sends timer events to the creator thread.
This resulted in the GUI thread's event loop getting compromised ( most of the times stopped processing it's own events ).
I am not sure if you remember it, but it was about the same time when I had problems with that crash in QProgressDialog.
Anyway, to make a long story short ( too late, I guess :) ), signals can be emitted from threads without event handlers but the other way around is not safe at all - actually it won't work( since we're talking about queued connection ).



Please note that this doesn't concern the finished() signal as it is emitted from the parent (where QThread object was created) thread.
Good to know that.

Regards

wysota
20th May 2007, 20:41
Not so sure about that. Signals can be emitted from within the thread to other threads - this works because the are nothing but some events. It is similar to calling QApplication: postEvent( someThread, someEvent ).
I'm talking about using signals within a thread, but this generally applies to using slots across threads as well.


Yes, any event that targets this thread( without an event loop ) is a potential danger to the entire application ( therefore to other threads ).
If a thread doesn't have an event loop, all its objects' events won't ever be processed.

patrik08
20th May 2007, 21:35
Hurra :D :D now is run .... and i can take 10 or more remote dir ls -al on 800
millisecond. (lan)

the next QHttp i build direct inside QThread to suppress many class...




/*
on subclass QHttp i suppress close on this way ..
i observer that QThread wait for close ...
( opinion or mistake? whitout this close nothing run.. )
*/
void close()
{
deleteLater();
}



class DavListDir : public QThread
{
Q_OBJECT
public:
void run();
QString GetxmlData();
void SetUrl( QUrl u );
signals:
void statusMessage(QString);
void TimeEnd();
private:
QUrl urltogrep;
QString ned;
HTTP_propfind *jobb;
public slots:
void quit();

};
void DavListDir::quit()
{
ned = jobb->GetDAVData();
emit finished();
std::cout << "----- quit call..." << std::endl;
exit(0);
}
void DavListDir::SetUrl( QUrl u )
{
urltogrep = QUrl(u);
jobb = new HTTP_propfind(urltogrep); /* QHttp first setup header .... */
connect(jobb, SIGNAL(done(bool)), this, SLOT(quit()));
}

QString DavListDir::GetxmlData()
{
return ned;
}

void DavListDir::run()
{
if (!urltogrep.isValid()) {
quit();
return;
}
jobb->start();
exec(); /* QHttp start request */
forever{
if (isFinished()) {
ned = jobb->GetDAVData();
emit finished();
}
};
}

wysota
20th May 2007, 21:57
Hmmm.... This code is.... well... how should I say that.... hmm... incorrect :)
This way the thread will never terminate... What's the point of spinning in the forever loop?

marcel
20th May 2007, 22:04
You shouldn't emit finished() from the thread.
Wysota said that the parent must emit this signal - seem logical.

Anyway, the code in run() that comes after exec() gets called only when you call quit() or exit().
I suppose it is correct, but you shouldn't emit finished() here. Just let the thread exit from run() by itself, and it's parent will emit finished() when appropriate.

Regards.

marcel
20th May 2007, 22:10
Hmmm.... This code is.... well... how should I say that.... hmm... incorrect :)
This way the thread will never terminate... What's the point of spinning in the forever loop?
It will finish, but only if he calls quit() outside the thread :)...This is one crazy thread.

Oh, BTW QThread::quit() is not virtual, neither is exit. So I'm not sure how correct it is to reimplement it. Anyway, if qt uses your class, it will call the base class version.

On the other hand, if you use it on an instance of your class, it will get called ok.
Why not rename it to something else, and call exit or quit in that function, as you do now.

patrik08
20th May 2007, 22:15
Hmmm.... This code is.... well... how should I say that.... hmm... incorrect :)
This way the thread will never terminate... What's the point of spinning in the forever loop?

not enter on forefer ... i delete...

marcel
20th May 2007, 22:20
Well, OK then.
But remember, even if the code works right now, it is not conceptually correct.
So, when you have the time, you should do a small redesign of your thread subclass.

Regards

patrik08
20th May 2007, 22:25
It will finish, but only if he calls quit() outside the thread :)...This is On the other hand, if you use it on an instance of your class, it will get called ok.
Why not rename it to something else, and call exit or quit in that function, as you do now.

is running ....



class DavListDir : public QThread
{
Q_OBJECT
public:
void run();
void SetUrl( QUrl u );
signals:
void DataXml(QString);
/*void TimeEnd();*/
private:
QUrl urltogrep;
QString ned;
HTTP_propfind *jobb;
public slots:
void grepdata();

};
void DavListDir::grepdata()
{
ned = jobb->GetDAVData();
if (ned.size() > 0) {
emit DataXml(ned);
exit(0);
}
}
void DavListDir::SetUrl( QUrl u )
{
urltogrep = QUrl(u);
jobb = new HTTP_propfind(urltogrep); /* QHttp first setup header .... */
connect(jobb, SIGNAL(done(bool)), this, SLOT(grepdata()));
}
void DavListDir::run()
{
if (!urltogrep.isValid()) {
quit();
return;
}
jobb->start(); /* QHttp start request */
exec();
forever{
if (isFinished()) {
emit grepdata();
}
};
}

wysota
20th May 2007, 22:27
It will finish, but only if he calls quit() outside the thread :)...
It probably won't emit finished() then.


Oh, BTW QThread::quit() is not virtual, neither is exit.
Every slot is "implicitely" virtual if redeclared in a subclass. That's a side effect of how signals and slot mechanism works.


So I'm not sure how correct it is to reimplement it. Anyway, if qt uses your class, it will call the base class version.
No, it'll call the "youngest" implementation that has been declared as a slot. Of course this only applies to a situation when the method is called as a result of signal emission and not directly.

marcel
20th May 2007, 22:30
is running ....



class DavListDir : public QThread
{
Q_OBJECT
public:
void run();
void SetUrl( QUrl u );
signals:
void DataXml(QString);
/*void TimeEnd();*/
private:
QUrl urltogrep;
QString ned;
HTTP_propfind *jobb;
public slots:
void grepdata();

};
void DavListDir::grepdata()
{
ned = jobb->GetDAVData();
if (ned.size() > 0) {
emit DataXml(ned);
}
}
void DavListDir::SetUrl( QUrl u )
{
urltogrep = QUrl(u);
jobb = new HTTP_propfind(urltogrep); /* QHttp first setup header .... */
connect(jobb, SIGNAL(done(bool)), this, SLOT(grepdata()));
}
void DavListDir::run()
{
if (!urltogrep.isValid()) {
quit();
return;
}
jobb->start(); /* QHttp start request */
exec();
forever{
if (isFinished()) {
emit grepdata();
}
};
}



No, not OK.
isFinished always returns false when called within thread ( especially from run ).
:)

patrik08
20th May 2007, 22:48
this run .... && having time out if loop....

is this better ....?




class DavListDir : public QThread
{
Q_OBJECT
public:
void run();
void SetUrl( QUrl u );
signals:
void DataXml(QString);
void TimeEnd();
private:
QUrl urltogrep;
QString ned;
HTTP_propfind *jobb;
bool waiting;
int decimalsec;
public slots:
void grepdata();

};
void DavListDir::grepdata()
{
ned = jobb->GetDAVData();
if (ned.size() > 0) {
emit DataXml(ned);
waiting = false;
quit();
}
}
void DavListDir::SetUrl( QUrl u )
{
decimalsec = 0;
waiting = true;
urltogrep = QUrl(u);
jobb = new HTTP_propfind(urltogrep); /* QHttp first setup header .... */
connect(jobb, SIGNAL(done(bool)), this, SLOT(grepdata()));
}
void DavListDir::run()
{
if (!urltogrep.isValid()) {
quit();
return;
}
jobb->start(); /* QHttp start request */
exec();

while( waiting ) {
decimalsec++;
msleep(100);
/* time out remote server! 2.5 sec .*/
if (decimalsec > 25) {
waiting = false;
ned ="time_out";
emit DataXml(ned);
quit();
}
}

}

marcel
20th May 2007, 22:58
No Patrick.
Think at exec() as a loop ( actually it is one :)) ). This loop exits when you call quit or exit.
When the loop exits run() will continue with the next instructions - meaning the code that you have after exec().

So there is no point in calling quit there. You shouldn't call emit either. Just pout the thread to sleep for as long as you have to.


while(wainting)
msleep( 200 );

You set waiting to false with from the GUI thread with a function that does only that : sets waiting to false( make sure "waiting" is volatile bool ). Therefore create another function in your thread that sets waiting to false.
You won't need grepdata, or anything else.
In the GUI thread, first call thread->quit(), and then, after some time( you must know when ), set waiting to false by calling that function.

When it exits, it will emit finished(). In the slot that gets called by finished you must query the thread for "ned". This seems a reasonable solution to me.

wysota
20th May 2007, 23:06
What does the while(waiting) loop do anyway? If you're spinning in a loop, why not use the one that is handled by exec()? You can use timers if you want to do something periodically. Seems that all you want is to react after 2.5s, so you can use a timer with such timeout and don't bother complicating things too much. And you can always stop the timer if you want to prevent it from timing out.

patrik08
20th May 2007, 23:39
I tested this on bad url bad pass and correct param url is running ... on max 2sec. one dir list... one a clean xml or a message error..

IMO: If troltech group know that the FTP password are visible clear, on place from QFtp QWebdav is born... and a remote dir model... must not write now..

if you see other code leaks? ...



class DavListDir : public QThread
{
Q_OBJECT
public:
void run();
void SetUrl( QUrl u );
signals:
void DataXml(QString);
void TimeEnd();
private:
QUrl urltogrep;
QString ned;
HTTP_propfind *jobb;
bool waiting;
int decimalsec;
public slots:
void grepdata();
void httperror();

};
void DavListDir::httperror()
{
emit DataXml(jobb->errorString());
jobb->abort();
exit();
}
void DavListDir::grepdata()
{
ned = jobb->GetDAVData();
if (ned.size() > 0) {
emit DataXml(ned);
waiting = false;
exit();
return;
}
QTimer::singleShot(2000, this, SLOT(httperror()));
}
void DavListDir::SetUrl( QUrl u )
{
decimalsec = 0;
waiting = true;
urltogrep = QUrl(u);
jobb = new HTTP_propfind(urltogrep); /* QHttp first setup header .... */
connect(jobb, SIGNAL(done(bool)), this, SLOT(grepdata()));
}
void DavListDir::run()
{
if (!urltogrep.isValid()) {
waiting = false;
ned ="url_invalid";
emit DataXml(ned);
exit();
return;
}
jobb->start(); /* QHttp start request */
exec();
}

wysota
21st May 2007, 00:02
I think your code is not very well designed, but if it works for you... hey, it's your application.

patrik08
21st May 2007, 00:22
I think your code is not very well designed, but if it works for you... hey, it's your application.

Then pleas tell my more, it would be to better read or designed, my realy job is kitchen chief... not a qt professional. And i stay here to learn... .. I am only a php guru & webmaster.. on QT book from Molkentin or Blanchette dont say nothing how make code better readable .. Or tell me from all here http://code.google.com/hosting/search?q=label:Qt 94 qt projekt which is better designed. tanks.

wysota
21st May 2007, 01:12
It's not only about readability, but also performance and ease of maintenance. I won't design your code because simply I don't know what you want it to do :) I can give you hints how to obtain a better design and I think I've already given a few in this thread.

patrik08
21st May 2007, 02:12
Witold, My actual book are UML design plattern on italian language
http://www.hoepli.it/libro.asp?ty=&id=302082&pc=000022007003007&mcs=0
not only Jasmin Blanchette on german...

But all this sample from Sudoku or raw C++ sample dont have any comparison from QT... on C++ any small function must self write to compose a class.. yesterday i look the code from Mac http://webdav.org/goliath/ (2002 last commit) .. to pick idea to my CMS webdav projekt. It have 5 file to build a Thread and Fork to get a listing from dir view http Propfind method 1200 or more line of code.. and i make the same class on 280 line qt code..

What i want to say on QT design plattern ist not simple if you can join a QThread , a QMainwindow , a Qhttp and 3 button and play all on 2 or 1 QTreeView like my projekt http://sourceforge.net/projects/qt-webdav/ ...

On this day i work on a big projekt .. an moore i write and going forward the difficulty was chance from the first day from projekt... I write my class on a simple main .. and if is running ok ... i put the class on a static lib path... at end i have only one or two libs and a QMainwindow , this is my actualy design plattern. I do not know other method.

wysota
21st May 2007, 08:51
It's not about design patterns. It's about such simple things as using the event loop correctly :) In your situation I don't know why you are using threads at all...