QHttp in thread (QT4)

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

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

To copy to clipboard, switch view to plain text mode 

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

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

To copy to clipboard, switch view to plain text mode 

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

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

To copy to clipboard, switch view to plain text mode 

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

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

To copy to clipboard, switch view to plain text mode 

If you want to delete the file, just call

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

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

To copy to clipboard, switch view to plain text mode 

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

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

To copy to clipboard, switch view to plain text mode 

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

int index = httpPointers.indexOf(sendHttp);
httpPointers.removeAt(index);

To copy to clipboard, switch view to plain text mode 

I think you can (but don't have to - your code should be slightly faster) substitute it with:

httpPointers.removeAll(sendHttp);

httpPointers.removeAll(sendHttp);

To copy to clipboard, switch view to plain text mode 

[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