Manage periodic tasks with QNetworkAccessManager

TheEnigmist

I've a periodic task that run each day that send HTTP requests. I create a thread for those tasks but after some days it crashes at reply = networkaccessmanager.get(). Maybe it crashes due to creating a NAM each day for each thread. It is possible that creating more NAM will crash the app?
Btw I tried to use one single NAM but it fails to generate a reply due to "Thread parent error" (create a reply in a different thread wich NAM was created) .
I made this due to send HTTP request in parallel, but now I'm reading that NAM will execute 6 parallel request (for desktop).
So what is the best practice for manage this scenario?
I'm thinking to create a Singleton NAM and each thread send signal to it for sending requests, but how can I connect reply back to the correct thread?

aha_1980

@TheEnigmist

Do you really need to have the NAM in threads?

The longest time should be getting the data from network, and thanks to Signals&Slots that should not block your program.

You can then emit the data to different threads for processing, if needed. But maybe that processing don't need multiple threads too?

Regards

TheEnigmist

@aha_1980 said in Manage periodic tasks with QNetworkAccessManager:

@TheEnigmist

Do you really need to have the NAM in threads?

The longest time should be getting the data from network, and thanks to Signals&Slots that should not block your program.

You can then emit the data to different threads for processing, if needed. But maybe that processing don't need multiple threads too?

Regards

Not really. I'm editing my project so I'll use only 1 NAM for entire program. But I'm facing a problem I don't know how to solve.
I created my NAM with 1 signal and 1 slot like this:

SIGNAL

void finished(Status, const QString &);

SLOT (I don't really know if I need it to be a slot or a normal method)

sendRequest(const QNetworkRequest & request, Method method, const QByteArray & data) {
	QNetworkReply *reply;

	switch (method) {
		case Network::GET:
			reply = nam->get(request);
			break;
		case Network::POST:
			reply = nam->post(request, data);
			break;
		default:
			reply = nam->get(request);
			break;
	}

	connect(reply, &QNetworkReply::finished, [this, reply] () {
		if (reply->error())
			emit finished(Network::ERROR, QString("%1 %2").arg(QString::number(reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt()), reply->errorString()));
		else
			emit finished(Network::OK, QString(reply->readAll()));
		reply->deleteLater();
	});
}

Now, when I use it, I connect finished signal to thread I need, but, as expected, the response will be caught from all threads and not only by the one I need.

connect(network, &Network::finished, [=] (Network::Status status, const QString & reply) {
   //Thread specific behaviour
}

There is a way to disconnect this connection after it reach the lambda his end?

EDIT
Maybe I found a solution, I added this line at the end of lambda function:

disconnect(network, &Network::finished, 0, 0);

I disconnect everything connected to Network so I'm sure each request will trigger 1 finished connection

Is it a good solution or can I improve it?

SGaist

Hi,

@TheEnigmist said in Manage periodic tasks with QNetworkAccessManager:

Now, when I use it, I connect finished signal to thread I need, but, as expected, the response will be caught from all threads and not only by the one I need.

What exactly do you mean by that ? It is not clear based on your current code snippet.

TheEnigmist

@SGaist said in Manage periodic tasks with QNetworkAccessManager:

Hi,

@TheEnigmist said in Manage periodic tasks with QNetworkAccessManager:

Now, when I use it, I connect finished signal to thread I need, but, as expected, the response will be caught from all threads and not only by the one I need.

What exactly do you mean by that ? It is not clear based on your current code snippet.

Sorry, I try to be more clear.
I use it for all my HTTP requests, and I use connect even if it will be a one time request. After the response the connection is still alive and if I forget to disconnect it, when Network emits finished it trigger that connection again, so I solved it with:

disconnect(network, &Network::finished, 0, 0);

And I'm asking if it is good or there is a better way to manage single NetworkAccessManager.

EDIT:

Another question is for multiple requests, f.e. if I send a HTTP request and then based on response I need to send another request, do I have to use connect() inside first connect()? Something like this:

network->sendRequest(firstRequest, Network::GET);
connect(network, &Network::finished, [=] (Network::Status status, const QString & reply) {
      disconnect(network, &Network::finished, 0, 0); //prevent triggering again
      /*
      parse reply
     */
     network->sendRequest(secondRequest, Network::GET);
     connect(network, &Network::finished, [=] (Network::Status status, const QString & reply) {
              disconnect(network, &Network::finished, 0, 0); //prevent triggering again
      }

When I use that Network class with parallel threads they get response from all other threads, how can I get the correct response for each thread? Actually I've connected them like this:

connect(this, &ThreadWorker::sendRequest, network, &Network::sendRequest);
connect(network, &Network::finished, this, &ThreadWorker::response); // <- this will output the same response (I suppose first) instead of expected thread  specific response

SGaist

Would it be possible to look at the complete implementation ?

From your snippet, it is not really clear.

TheEnigmist

@SGaist
Sorry for delay. I made a zip with a functional program that simulate my huge project behaviour. The issue is the same as explained before.
This little extract will send an echo POST but different thread gather the same response (not the one they sent)
You can download source from here

EDIT:
Maybe I found a solution. Adding an identifier to Network slot/signal and in each thread slot check if sendend ID match received signal ID. If true do action and terminate (disconnecting too) otherwise do nothing!

SGaist

One of the issue you have with your design is that you are not making it easy to use with multiple threads. If you want a specific thread to handle a specific answer then you should either associate some sort of ID with the answer or trigger the thread when the reply is finished.

Out of curiosity, why are you making a QString from your reply content to transform it back to a QByteArray in your processing method ?

TheEnigmist

@SGaist
Yes I think I will uso QUuid as sender ID and if it match use that reply content.
Btw I make QString from Network reply due to error handling (number + error string) but I think I can just cast it back to QByteArray so I can change signal definition with QByteArray instead of QString

EDIT:
Out of curiosity, what will be a good design for easy use with threads?

SGaist

I would start the worker object upon reception of the data.