QThread exec proplem to stop...

[marcel]

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]

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]

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

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

To copy to clipboard, switch view to plain text mode 

[wysota]

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]

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]

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

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

To copy to clipboard, switch view to plain text mode 

[marcel]

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

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

To copy to clipboard, switch view to plain text mode 

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

[patrik08]

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

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

To copy to clipboard, switch view to plain text mode 

[marcel]

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

while(wainting)
 msleep( 200 );

To copy to clipboard, switch view to plain text mode 

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]

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]

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

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

To copy to clipboard, switch view to plain text mode 

[wysota]

I think your code is not very well designed, but if it works for you... hey, it's your application.

[wysota]

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.

[patrik08]

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]

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

[marcel]

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.