Details
-
Bug
-
Resolution: Unresolved
-
P3: Somewhat important
-
None
-
4.6.3
-
None
-
Linux, x86_64
Description
On Qt 4.6.3, the TelepathyQt4 unit tests report memory leaks from code such as:
void ClientHandlerRequestsAdaptor::RemoveRequest( const QDBusObjectPath &request, const QString &errorName, const QString &errorMessage, const QDBusMessage &message) { /* ... */ mBus.send(message.createReply()); /* ... */ }
The unit tests have the service and the client in the same process, so this code path is triggered in QDbusMessage::createReply():
447 if (d_ptr->localMessage) { 448 reply.d_ptr->localMessage = true; 449 d_ptr->localReply = new QDBusMessage(reply); // keep an internal copy 450 }
Valgrind reports the leak from line 449, from the internal copy creation. What's confusing is that QDBusMessagePrivate::~QDBusMessagePrivate DOES in fact delete localReply - which indicates that the private struct itself is not deleted in some situations, OR localReply in it is overwritten for some reason. The only way I could see that happening is create(Error)Reply being called twice. It seems that Qt does another attempt at creating a reply because the code doesn't use message.setDelayedReply(true) - sure enough, if I insert that before the send() line, the memory leak is gone. However, I don't feel like setting the reply to delayed should be required - the reply is sent already before the method returns, hence it's not really delayed, and it shouldn't be required to explicitly tell Qt to not send a reply.
Therefore, I suggest one or more of the following:
1) Don't try to double-create replies if a reply is already sent, even if setDelayedReply() is not used
2) Free any existing localReply in QDBusMessage::create(Error)Reply()
OR, if you really feel that setDelayedReply should be used in this case:
3) Disallow createReply() if setDelayedReply(true) hasn't been called
OR
4) Make createReply() automatically do setDelayedReply(true)