QNetworkAccessManager reusing connections -- breaks

poncho524

Why is QNetworkAccessManager pipelining when it is told not to?

I'm trying to send a sequence of POST's, and chaining them based on the previous' QNetworkReply::finished().

On QNetworkReply::finished(), i "deleteLater" the reply and issue the next POST. The next POST always fails because it is being pipelined and "deletelater" on the PREVIOUS reply closes the connection.

I'm guessing this is a undocumented "feature".

anyone have tips?

ChrisW67

@poncho524

HTTP Pipelining is the sending of multiple HTTP/1.1 requests on the same connection without waiting for their corresponding responses. Your code explicitly waits for a response and does not send a second request until the first is finished (so QNAM cannot be pipelining behind your back). I think pipelining is irrelevant to your problem. You can check for the attribute QNetworkRequest::HttpPipeliningWasUsedAttribute on the reply to confirm this.

QNetworkAccessManager will reuse connections for multiple requests to the same server/port or use multiple connections to the same server to process queued requests. Once again, you serialise requests, so the queue is at most one long and parallel execution is also a non-issue.

Your real issue is that you delete the wrong reply here:

void HttpClient::processFirstDone()
{
  emit httpClientReady();
  m_reply->deleteLater();
}

The slot called when you emit that signal is executed immediately and overwrites m_reply. When the signal returns you then queue deletion of the new reply. Swap the two lines and things will probably improve.

SGaist

Hi,

Can you show the code you use ?

artwaw

@poncho524 Please show your code, I never encountered similar problem?

poncho524

Wireshark shows both POST's being sent via the same connection. it also shows the Qt app sending a FIN ACK before the 2nd POST is completed. this breaks the signals coming back from the 2nd POST... "processNextDone" and "processNextError" never get called.

void HttpClient::somefunction()
{
  qNam=new QNetworkAccessManager;

  QNetworkRequest req(QUrl("http://someip/somepath"));
  req.setHeader(QNetworkRequest::ContentTypeHeader, "application/json");

  QJsonDocument doc;
  QJsonObject obj;
  obj.insert("somefield", "somevalue");

  doc.setObject(obj);

  m_reply=qNam->post(req, doc.toJson());

  connect(m_reply, &QNetworkReply::finished, this, &HttpClient::processFirstDone);
  
  connect(this, &HttpClient::httpClientReady, this, &HttpClient::sendNext);
}


void HttpClient::processFirstDone()
{
  emit httpClientReady();
  m_reply->deleteLater();
}


void HttpClient::sendNext()
{
  QNetworkRequest req(QUrl("http://someip/someotherpath"));
  req.setHeader(QNetworkRequest::ContentTypeHeader, "application/json");

  QJsonDocument doc;
  QJsonObject obj;
  obj.insert("somefield", "somevalue");

  doc.setObject(obj);

  m_reply=qNam->post(req, doc.toJson());

  connect(m_reply, &QNetworkReply::finished, this, &HttpClient::processNextDone);
  connect(m_reply, &QNetworkReply::errorOccurred, this, &HttpClient::processNextError);
}


// this never gets called
void HttpClient::processNextDone()
{
  printf("processNextDone()\n");
}

// this never gets called
void HttpClient::processNextError()
{
  printf("processNextDone()\n");
}

poncho524

this is with QT 5.15.2

poncho524

https://doc.qt.io/qt-5/qnetworkrequest.html#Attribute-enum

QNetworkRequest::HttpPipeliningAllowedAttribute
Requests only, type: QMetaType::Bool (default: false) Indicates whether the QNetworkAccessManager code is allowed to use HTTP pipelining with this request.

ChrisW67

@poncho524

HTTP Pipelining is the sending of multiple HTTP/1.1 requests on the same connection without waiting for their corresponding responses. Your code explicitly waits for a response and does not send a second request until the first is finished (so QNAM cannot be pipelining behind your back). I think pipelining is irrelevant to your problem. You can check for the attribute QNetworkRequest::HttpPipeliningWasUsedAttribute on the reply to confirm this.

QNetworkAccessManager will reuse connections for multiple requests to the same server/port or use multiple connections to the same server to process queued requests. Once again, you serialise requests, so the queue is at most one long and parallel execution is also a non-issue.

Your real issue is that you delete the wrong reply here:

void HttpClient::processFirstDone()
{
  emit httpClientReady();
  m_reply->deleteLater();
}

The slot called when you emit that signal is executed immediately and overwrites m_reply. When the signal returns you then queue deletion of the new reply. Swap the two lines and things will probably improve.

poncho524

@ChrisW67 , oh man, i feel like a jerk now. nice catch!

ChrisW67

We've all been there (or we are not trying hard enough).

JonB

@ChrisW67 said in QNetworkAccessManager reusing connections -- breaks:

Your real issue is that you delete the wrong reply here:
void HttpClient::processFirstDone()
{
emit httpClientReady();
m_reply->deleteLater();
}

The slot called when you emit that signal is executed immediately and overwrites m_reply. When the signal returns you then queue deletion of the new reply. Swap the two lines and things will probably improve.

Hi Chris, this is a question for you, as you know more than I.

It worries me when you say to put the deleteLater() first. The reply must remain valid till this slot exits and returns, right? And the delete will happen as soon as the main event loop gets a chance to do so. How do you know what emitting the httpClientReady signal might cause to happen in the outside world? It could cause the main loop to be reached and the delete to happen, yes or no?

So.... would the following pattern be "safer"?

void HttpClient::processFirstDone()
{
  QNetworkReply *oldReply = m_reply;
  emit httpClientReady();
  oldReply->deleteLater();
}

JKSH

@JonB said in QNetworkAccessManager reusing connections -- breaks:

The reply must remain valid till this slot exits and returns, right?

Right.

And the delete will happen as soon as the main event loop gets a chance to do so.

I'm pretty sure the delete will happen after the main event loop finishes processing everything else in the event queue, not "as soon as it gets a chance".

It could cause the main loop to be reached and the delete to happen, yes or no?

No. After the event loop passed control to HttpClient::processFirstDone(), it cannot receive control again until HttpClient::processFirstDone() returns.

If emit httpClientReady() has a Qt::DirectConnection, the connected slot will be called from inside HttpClient::processFirstDone() (so the event loop is not involved). If emit httpClientReady() has a Qt::QueuedConnection, the connected slot can't run until after HttpClient::processFirstDone() returns.

JonB

@JKSH
The question is what could happen if a programmer puts a QApplication::ProcessEvents() in whatever this slot calls (whatever emitting httpClientReady() causes)? And we know how many users like to call this, with or without a local QEventLoop....

Now, it may be that this comes down to https://doc.qt.io/qt-5/qcoreapplication.html#processEvents and what it says about DeferredDelete events, which I believe is what deleteLater() calls/uses?

In the event that you are running a local loop which calls this function continuously, without an event loop, the DeferredDelete events will not be processed. This can affect the behaviour of widgets, e.g. QToolTip, that rely on DeferredDelete events to function properly. An alternative would be to call sendPostedEvents() from within that local loop.

So, please, I invite you to comment, because this is an area I have been unsure about. Are you saying Qt is saying that even with some QApplication::processEvents() somewhere Qt will not act on a pending deleteLater()? That is my area of concern.

Christian Ehrlicher

QCoreApplication::processEvents() does not run the event loop, it just processes events so QObject::deleteLater() will not be called in there ('The object will be deleted when control returns to the event loop.') :)
Technically deleteLater is also only an event but it's handled specially like this comment in qcoreapplication.cpp show us:

// Events sent by non-Qt event handlers (such as glib) may not
// have the scopeLevel set correctly. The scope level makes sure that
// code like this:
//     foo->deleteLater();
//     qApp->processEvents(); // without passing QEvent::DeferredDelete
// will not cause "foo" to be deleted before returning to the event loop.

JonB

@Christian-Ehrlicher
Thank you, helpful. So to be clear:

control returns to the event loop

There is just one, unique main Qt event loop running? (Or maybe more precisely one per thread?)
User cannot write code that, from a slot/anywhere, itself can enter that loop?