1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

QRestAccessManager: don't leak slot objects when no QNAM was set

Browse Source 
We try to pass QSlotObjectBase as raw pointer through the ABI
boundary, because we hope to have the template-wrapper tail-call the
out-of-line helper¹. But that means that the out-of-line helper needs
to ensure that it deletes the slot object on every possible exit from
the function. The old code didn't do that e.g. when the qnam check
failed, or, theoretically, if requestOperation() threw an exception.

The new code places the slot object under SlotObjUniquePtr guard as
soon as possible (but, due to amount of callers, not before
executeRequest()). Port createActiveRequest() to receive the slot
object already in a smart pointer. Tail-calling isn't required here,
because caller and callee are part of the same TU so the optimizer can
do whatever it wants. Unique_ptr passing shouldn't be that hard to
optimize.

¹ If we were to pass by SlotObjUniquePtr, tail-calling would be
  impossible due to the need to run the unique_ptr's dtor after he
  call to the helper.

Amends 9ba5c7ff6aa42c5701cf950d2137467a2d178833, but
e560adef213301318dcc13d4db155624846e0420 already had the
requestOperation() problem.

Pick-to: 6.7
Change-Id: I2ab5eadb35625393f274e3391d7b7c393ed8f08a
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
(cherry picked from commit 11725d46344105325a1dec8628708ab88483ca12)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
Marc Mutz 2024-07-04 14:21:15 +02:00 committed by Qt Cherry-pick Bot
parent e37832cc37
commit 056c89e95d
2 changed files with 9 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/network/access/qrestaccessmanager.cpp
View File

@ -755,11 +755,11 @@ QNetworkReply *QRestAccessManager::customWithDataImpl(const QNetworkRequest &req
QNetworkReply *QRestAccessManagerPrivate::createActiveRequest(QNetworkReply *reply,
const QObject *contextObject,
QtPrivate::QSlotObjectBase *slot)
QtPrivate::SlotObjUniquePtr slot)
{
Q_Q(QRestAccessManager);
Q_ASSERT(reply);
QtPrivate::SlotObjSharedPtr slotPtr(QtPrivate::SlotObjUniquePtr{slot}); // adopts
QtPrivate::SlotObjSharedPtr slotPtr(std::move(slot)); // adopts
activeRequests.insert(reply, CallerInfo{contextObject, slotPtr});
// The signal connections below are made to 'q' to avoid stray signal
// handling upon its destruction while requests were still in progress

12
src/network/access/qrestaccessmanager_p.h
View File

@ -36,18 +36,19 @@ public:
~QRestAccessManagerPrivate() override;
QNetworkReply* createActiveRequest(QNetworkReply *reply, const QObject *contextObject,
QtPrivate::QSlotObjectBase *slot);
QtPrivate::SlotObjUniquePtr slot);
void handleReplyFinished(QNetworkReply *reply);
using ReqOpRef = qxp::function_ref<QNetworkReply*(QNetworkAccessManager*) const>;
QNetworkReply *executeRequest(ReqOpRef requestOperation,
const QObject *context, QtPrivate::QSlotObjectBase *slot)
const QObject *context, QtPrivate::QSlotObjectBase *rawSlot)
{
QtPrivate::SlotObjUniquePtr slot(rawSlot);
if (!qnam)
return warnNoAccessManager();
verifyThreadAffinity(context);
QNetworkReply *reply = requestOperation(qnam);
return createActiveRequest(reply, context, slot);
return createActiveRequest(reply, context, std::move(slot));
}
using ReqOpRefJson = qxp::function_ref<QNetworkReply*(QNetworkAccessManager*,
@ -55,8 +56,9 @@ public:
const QByteArray &) const>;
QNetworkReply *executeRequest(ReqOpRefJson requestOperation, const QJsonDocument &jsonDoc,
const QNetworkRequest &request,
const QObject *context, QtPrivate::QSlotObjectBase *slot)
const QObject *context, QtPrivate::QSlotObjectBase *rawSlot)
{
QtPrivate::SlotObjUniquePtr slot(rawSlot);
if (!qnam)
return warnNoAccessManager();
verifyThreadAffinity(context);
@ -68,7 +70,7 @@ public:
}
req.setHeaders(std::move(h));
QNetworkReply *reply = requestOperation(qnam, req, jsonDoc.toJson(QJsonDocument::Compact));
return createActiveRequest(reply, context, slot);
return createActiveRequest(reply, context, std::move(slot));
}
void verifyThreadAffinity(const QObject *contextObject);