9

I have doubts how should I use QEventLoop. I have 2 pieces of code, both of them work for me (get web resource downloaded).

First one:

QNetworkAccessManager *manager = new QNetworkAccessManager( this );
QNetworkRequest request;
request.setUrl(QUrl(url));
request.setRawHeader("User-Agent", "Mozilla Firefox");
connect(manager, SIGNAL(finished(QNetworkReply*)),this,SLOT(replyFinished(QNetworkReply*)));
manager->get( request )  ;

QEventLoop loop;
connect(manager, SIGNAL(finished(QNetworkReply*)),&loop, SLOT(quit()));
loop.exec();

Second one:

QNetworkAccessManager *manager = new QNetworkAccessManager( this );
QNetworkRequest request;
request.setUrl(QUrl(url));
request.setRawHeader("User-Agent", "Mozilla Firefox");
manager->get( request )  ;

QEventLoop loop;
connect(manager, SIGNAL(finished(QNetworkReply*)),this, SLOT(replyFinished(QNetworkReply*)));
loop.exec();

What I want to know is which one should I use. I mean, does the event loop quit in the second one after signal is emmited? Or do I have to call quit() like in the first one? I found the second solution somewhere but it didn't seem proper to me so I modified it into first piece of code.

Erik
  • 2,137
  • 3
  • 25
  • 42
Bartek Boczar
  • 257
  • 1
  • 3
  • 7
  • How do you want to interrupt event loop in second case? First is OK, but you should handle errors too. – Dmitry Sazonov Apr 04 '15 at 17:52
  • Yeah, thats what i was worring abou so I changed it. I just wasnt sure if I was thinking right way so i asked – Bartek Boczar Apr 04 '15 at 18:04
  • 2
    In general you shouldn't be using either -- QApplication already sets up an event loop for the main thread, and QThread sets up an event loop for background threads. – MrEricSir Apr 04 '15 at 18:25
  • If you add an extra connection to terminate event loop in second case, then it will be the same with first case. What is your question about? – Dmitry Sazonov Apr 04 '15 at 18:26
  • @MrEricSir you wrong. `QEventLoop` is designed for such cases. When you don't want to make your code complex (with a lot of signals/slots) and need a single flow with support of event-driven logic. – Dmitry Sazonov Apr 04 '15 at 18:27
  • 5
    Local QEventLoops are the root of all evil. (as all kind of things can happen before loop.exec() returns). Connect finished to another slot, and continue there. – Frank Osterfeld Apr 04 '15 at 20:44
  • 1
    I agree to Frank, using an eventloop for this case seems over the top and might create complex issue – Damien Sep 21 '16 at 05:23

2 Answers2

4

I agree with @Mher-Didaryan - that the event loop started by following line of code loop.exec(); in the 2nd code snippet - will never exit. This is because the connect() between the SIGNAL and SLOT is being done for a different event loop than the event loop indicated through EventLoop loop; .

In the case of the 1st code snippet, the logic depends on the finished(QNetworkReply*) signal associated with one & same GET request being emitted to two different event loops. But it is quite possible that

    connect(manager, SIGNAL(finished(QNetworkReply*)),&loop, SLOT(quit()));

may well execute after the manager->get( request ) ; has emitted the finished(QNetworkReply*) signal. Maybe it can happen for a GET type HTTP operation involving a very small file or response. In such a scenario the event loop started out by the loop.exec(); in the 1st code snippet will also not exit. I guess this is what @Mher-Didaryan is also querying in his answer.

Maybe you can use the below QEventLoop logic that would handle the following negative execution scenarios too

  1. Timing out of the GET request (say due to network connectivity issues)
  2. Error type response from server side of network

    QNetworkAccessManager *manager = new QNetworkAccessManager(this);
    QNetworkRequest request;
    QEventLoop loop;
    QTimer getTimer; // let's use a 10 second period for timing out the GET opn
    request.setUrl(QUrl(url));
    request.setRawHeader("User-Agent", "Mozilla Firefox");
    // connect the timeout() signal of getTimer object to quit() slot of event loop
    QTimer::connect(&getTimer,SIGNAL(timeout()),&loop, SLOT(quit()));
    QObject::connect(manager, SIGNAL(finished(QNetworkReply*)),&loop, SLOT(quit()));
    QNetworkReply *resp = manager->get( request );        
    getTimer.start(10000); // 10000 milliSeconds wait period for get() method to work properly
    loop.exec();
    
    if(NULL == resp)
    {
        // Error. we probably timed out i.e SIGNAL(finished()) did not happen
        // this handles above indicated case (1)
        return -1; // or return some timeout related error value
    }
    else if( QNetworkReply::NoError != resp->error() )
    {
        // Error - SIGNAL(finished()) was raised but get() opn failed & returned with error
        // Refer http://doc.qt.io/qt-4.8/qnetworkreply.html#NetworkError-enum
        // This section of code handles above indicated case (2)
    }
    else
    {
        // get() operation was Successful !.
        // read the response available in the 'resp' variable as a QString & parse it. 
        // Obtain the necessary result and etc.
    }
    
    delete resp;
    delete manager;
    
1

In your second example event loop will never quit, on the other hand in your first example the loop will quit when finished(QNetworkReply*) emits. But what if manager->get( request ); cause finished(QNetworkReply*) signal to be emited before you connect loop's quit to it?

QNetworkAccessManager *manager = new QNetworkAccessManager( this );
QNetworkRequest request;
QEventLoop loop;
request.setUrl(QUrl(url));
request.setRawHeader("User-Agent", "Mozilla Firefox");
connect(manager, SIGNAL(finished(QNetworkReply*)),this,SLOT(replyFinished(QNetworkReply*)));
connect(manager, SIGNAL(finished(QNetworkReply*)),&loop, SLOT(quit()));
manager->get( request )  ;

loop.exec();

And also you need to somehow handle situation where manager does not emit SIGNAL(finished(QNetworkReply*)) at all.

Mher Didaryan
  • 95
  • 2
  • 13