Commit 4eafe7c5 authored by Igor Kushnir's avatar Igor Kushnir
Browse files

CustomSchemeHandler: don't leak QNetworkReply objects

The QNetworkReply objects created in
CustomSchemeHandler::requestStarted() are never destroyed, not even at
KDevelop exit, even though they are used for a few seconds at most.

From the documentation for QNetworkReply::finished():
  Note: Do not delete the object in the slot connected to this signal.
  Use deleteLater().

From the documentation for QWebEngineUrlRequestJob::reply():
  The user has to be aware that device will be used on another thread
  until the job is deleted. In case simultaneous access from the main
  thread is desired, the user is reponsible for making access to device
  thread-safe for example by using QMutex. Note that the device object
  is not owned by the web engine. Therefore, the signal
  QObject::destroyed() of QWebEngineUrlRequestJob must be monitored.
  The device should remain available at least as long as the job exists.
  When calling this method with a newly constructed device, one solution
  is to make the device as a child of the job or delete itself when job
  is deleted, like this:
    connect(job, &QObject::destroyed, device, &QObject::deleteLater);

Destroy CustomSchemeHandler's QNetworkReply object once it is finished
and the QWebEngineUrlRequestJob object that uses it is destroyed.

Qt WebEngine uses the QNetworkReply object on another thread. But our
HelpNetworkReply appears to be free of data races because it logically
finishes in the constructor. It is possible to call setFinished(true)
and emit finished() in a 0 ms timer single shot from HelpNetworkReply's
constructor, but such changes may cause a regression with Qt WebKit.
parent 676fff87
Pipeline #193361 passed with stage
in 27 minutes and 4 seconds
......@@ -27,6 +27,7 @@
#else
#include <QNetworkRequest>
#include <QNetworkReply>
#include <QPointer>
#include <QWebEngineView>
#include <QWebEnginePage>
#include <QWebEngineSettings>
......@@ -338,7 +339,21 @@ public:
void requestStarted(QWebEngineUrlRequestJob *job) override {
const QUrl url = job->requestUrl();
auto reply = m_nam->get(QNetworkRequest(url));
auto* const reply = m_nam->get(QNetworkRequest(url));
// Deliberately don't use job as context in this connection: if job is destroyed
// before reply is finished, reply would be leaked. Using reply as context does
// not impact behavior, but silences Clazy checker connect-3arg-lambda (level1).
connect(reply, &QNetworkReply::finished, reply, [reply, job = QPointer{job}] {
// At this point reply is no longer written to and can be safely
// destroyed once job ends reading from it.
if (job) {
connect(job, &QObject::destroyed, reply, &QObject::deleteLater);
} else {
reply->deleteLater();
}
});
job->reply(reply->header(QNetworkRequest::ContentTypeHeader).toByteArray(), reply);
}
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment