QHttp in thread (QT4)

[maxpower]

I am writing a downloading helper application and I have successfully got the program to download a url that is drop into the window. Now I am trying to allow more then one download at a time. Currently if more then one item is dropped in, the other files wait to be downloaded. I believe the fix is to put by QHttp into threads so that each download is its own thread. Does that sound right? If so, how do i do it? I have never worked with threads before.

Thanks
mAx

[camel]

Just a note:
From my understanding of QHttp it would be sufficient if you just use 1 thread (might even be the main loop) but use multiple QHttp objects.

QHttp is asynchronous, i.e. it does not block and only works if there is data to be processed, but can only handle one download at the time (it queues the requests internally)

[maxpower]

I thought the same thing but how do I have multiple QHttp objects when the user could download as many files as they want? Would my program have to have a hard limit on downloads? Or, is there a way to attach a numeral to a object (QHttp http[0])? Also, if there were multiple QHttps, wouldn't I have to have multiple slots?

Thanks
mAx

[jpn]

I thought the same thing but how do I have multiple QHttp objects when the user could download as many files as they want? Would my program have to have a hard limit on downloads? Or, is there a way to attach a numeral to a object (QHttp http[0])? Also, if there were multiple QHttps, wouldn't I have to have multiple slots?
Each http request returns an unique identifier which can be later used to identify the request.

[wysota]

Each http request returns an unique identifier which can be later used to identify the request.

Yes, but this is only true for a single object. Identifiers will duplicate when multiple QHttp objects are used.

My suggestion is to use a QList of pointers to QHttp and introduce a sane limit on number of simultaneous downloads.

Using multiple threads won't help here in any way - you'll still need multiple QHttp objects as QHttp uses the event loop thus it can be handled by a single thread only.

[maxpower]

So do you mean like this:



QList<QHttp> httpPointers = &http


What about signals/slots, is it like this:



connect(httpPointers.at(0), SIGNAL(something()), this SLOT(somethingelse()));


..with a different signal for each pointer? I guess I don't understand how this gives me one QHttp object with a unique set of request ids.

Thanks
mAx

[wysota]

So do you mean like this:



QList<QHttp> httpPointers = &http

No, I mean like this:

QList<QHttp*> httpPointers;
httpPointers << new QHttp(...);


What about signals/slots, is it like this:



connect(httpPointers.at(0), SIGNAL(something()), this SLOT(somethingelse()));

Yes, but you can connect the signal before adding the http object to the list - so you can use pointers directly, it'll be safer this way :)


..with a different signal for each pointer? I guess I don't understand how this gives me one QHttp object with a unique set of request ids.

When each signal is emitted, in each slot you can get a pointer to an object emitting the signal by calling QObject::sender(). This way you get instant access to the QHttp object that emitted the signal and you can handle all objects from a single slot this way.

[maxpower]

Okay, I think I see what I need to do and I will give it a try. But can you explain the signal part again? I understand i only need on slot since from there I can determine which http instance called singaled but how do I add the signals before adding the http object to the list? In your example I am supposed to add a new QHttp object to the list, not an existing one. Or is it like this:



QList<QHttp*> httpPointers;
QHttp httpOne = new QHttp();
QHttp httpTwo = new QHttp();
connect(httpOne, SIGNAL(...), this, SLOT(...));
connect(httpTwo, SIGNAL(...), this, SLOT(...));
httpPointers<<httpOne << httpTwo;


If that is right, are you also saying that I have to create my "sane" about of QHttp ojects statically in the code and then just which one to use based on a counter (and if a previous object is down downloading)?

Thanks
mAx

[wysota]

You can have a queue. If you want to add a download and the list is shorter than X then create a new QHttp object, connect its signals, add it to the list and assign it a request to process (remember to remove it from the list and delete the object when you don't need it anymore so that new objects may be added). If the list already has X objects and you need to perform another download, simply assign the request to one of already existing downloaders (try to do it in a round robin fashion). When the QHttp object finished fetching one request, it'll continue with a next one.

[maxpower]

Well, my new code is working and I can download more then one file at a time. I still need to work on the queue part but first I am having a problem removing the QHttp object when the request is finished. Here is my code:



void SSDownloaderMWI::httpRequestFinished(int requestId, bool error)
{
QHttp *sendHttp = qobject_cast<QHttp *>(sender());
QList<QTableWidgetItem*> items = downloadsTW->findItems(QString::number(requestId), Qt::MatchFixedString);
if(items.count() != 0)
{
int workingRow = items.at(0)->row();
QFile *file = new QFile();
file->setFileName(downloadsTW->item(workingRow, 10)->text());
file->close();
QTableWidgetItem *finishedStatus = new QTableWidgetItem("completed");
if(error)
{
file->remove();
finishedStatus->setText("Error");
}

downloadsTW->setItem(workingRow, 4, finishedStatus);
QTableWidgetItem *requestIdItem = new QTableWidgetItem("");
downloadsTW->setItem(workingRow, 8, requestIdItem);
QMessageBox::information(this,"this",QString::number(httpPointers.count()));
int index = httpPointers.indexOf(sendHttp);
httpPointers.removeAt(index);
QMessageBox::information(this,"this",QString::number(httpPointers.count()));
//delete sendHttp;
}
}


index is valid as it returns 0 during my tests of only one download running. I get the following error when my program crashes under debug:



rogram received signal SIGSEGV, Segmentation fault.
[Switching to Thread -1223771584 (LWP 9087)]
0xb718ae23 in memmove () from /lib/tls/i686/cmov/libc.so.6


Any ideas on what I am doing wrong?

Thanks
mAx

[wysota]

First of all you can't delete a signal sender from within a slot, you have to call deleteLater(), so that Qt will delete the object after all slots have finished their job.

Second of all, what the point of this code?


QFile *file = new QFile();
file->setFileName(downloadsTW->item(workingRow, 10)->text());
file->close();
QTableWidgetItem *finishedStatus = new QTableWidgetItem("completed");
if(error)
{
file->remove();
finishedStatus->setText("Error");
}

If you want to delete the file, just call

QFile::remove(downloadsTW->item(workingRow, 10)->text());

Third of all I'd connect to the done() signal and remove the downloader from the queue there instead of in requestFinished.

As for why the program crashes, could you please display the backtrace (bt) after you get a segfault in the debugger?

[maxpower]

Well, I thought I needed to close the file properly to ensure my program did not keep it open. Is that not true? I moved the code I had before into a done slot and now I do not see that debug error any more. i will have to keep an eye on it to ensure that it does not return. Based on me no having to manually close the file, here is my new code (just realized though that since I have multiple http objects I can not simply look up the request id in my table to ensure I am working on the correct entry since there could be duplicates, right?):



void SSDownloaderMWI::httpRequestFinished(int requestId, bool error)
{
QHttp *http = qobject_cast<QHttp *>(sender());
int currentID = http->currentId();
QList<QTableWidgetItem*> items = downloadsTW->findItems(QString::number(currentID), Qt::MatchFixedString);
if(items.count() != 0)
{
int workingRow = items.at(0)->row();
QTableWidgetItem *finishedStatus = new QTableWidgetItem("completed");
if(error)
{
QFile::remove(downloadsTW->item(workingRow, 10)->text());
finishedStatus->setText("Error");
}

downloadsTW->setItem(workingRow, 4, finishedStatus);
QTableWidgetItem *requestIdItem = new QTableWidgetItem("");
downloadsTW->setItem(workingRow, 8, requestIdItem);
}
}

void SSDownloaderMWI::httpDone(bool error)
{
QHttp *sendHttp = qobject_cast<QHttp *>(sender());
QMessageBox::information(this,"this",QString::number(httpPointers.count()));
int index = httpPointers.indexOf(sendHttp);
httpPointers.removeAt(index);
QMessageBox::information(this,"this",QString::number(httpPointers.count()));
sendHttp->deleteLater();
}



Thanks
mAx

[wysota]

Well, I thought I needed to close the file properly to ensure my program did not keep it open. Is that not true?
It's true (although it's possible that QHttp closes it itself), but here you create a new file object instead of closing the one already open. You should get a pointer to a QIODevice you assigned when making the request and call close on it.


I moved the code I had before into a done slot and now I do not see that debug error any more. i will have to keep an eye on it to ensure that it does not return.
It shouldn't unless you start deleting the signalling object from a slot again.


I can not simply look up the request id in my table to ensure I am working on the correct entry since there could be duplicates, right?):
Every id is unique per object, so the pair (QHttp*, id) can't duplicate.

BTW. If you make a qobject_cast, you should check if it doesn't return 0. If you're sure that it is a QHttp* (like in this case), you can make a static_cast instead.



int index = httpPointers.indexOf(sendHttp);
httpPointers.removeAt(index);
I think you can (but don't have to - your code should be slightly faster) substitute it with:

httpPointers.removeAll(sendHttp);

[maxpower]

Okay, thanks for the help AGAIN. I made all the changes you suggested and used QHttp::currentDestinationDevice() to get the pointer to my QFile and close it. I create a new QHttp object each time a new download is added so I will have to find a way to match the calling QHttp object with its entry in the table.

Thanks again
mAx

[wysota]

Here is my version of the downloader.

[maxpower]

Thanks for all your help. I used a QQueue and Download object like the one you used. My program now downloads up to 30 files at a time (user configurable) and stores the file based on a list of suffixes and destinations (user configurable). Now I just got to add more error detection/correction and a Firefox plugin of some sort.

Thanks
mAx