Making ~SlaveInterface virtual, the connection created in Slave constructor is now deleted. (thanks @dfaure).
Changed to new connect syntax.
BUG: 396651
dfaure | |
ngraham |
Frameworks |
Making ~SlaveInterface virtual, the connection created in Slave constructor is now deleted. (thanks @dfaure).
Changed to new connect syntax.
BUG: 396651
The leaks of the bug report are gone.
Automatic diff as part of commit; lint not applicable. |
Automatic diff as part of commit; unit tests not applicable. |
src/core/slave.cpp | ||
---|---|---|
148 | Good catch, maybe it should be a QScopedPointer? |
It's deleted in https://phabricator.kde.org/source/kio/browse/master/src/core/slaveinterface_p.h$47
~SlaveInterfacePrivate should be virtual
You can remove the "hopefully" from the commit log. setConnection is not used anywhere in KIO, and couldn't be used from outside KIO, since Connection isn't an exported class. I just marked setConnection/connection as deprecated and to be removed in KF6.
"~SlaveInterfacePrivate should be virtual" sounds like the actual fix for this bug indeed.
I still don't know how to avoid this leak, but I guess this will be fixed with new syntax for QMetaObject::invokeMethod and similar.
==26857== 22,638 (216 direct, 22,422 indirect) bytes in 3 blocks are definitely lost in loss record 2,175 of 2,182 ==26857== at 0x4C2EB4F: operator new(unsigned long) (vg_replace_malloc.c:334) ==26857== by 0xD7E74CE: QObjectPrivate::connectImpl(QObject const*, int, QObject const*, void**, QtPrivate::QSlotObjectBase*, Qt::ConnectionType, int const*, QMet aObject const*) (qobject.cpp:4834) ==26857== by 0xD7E78AC: QObject::connectImpl(QObject const*, void**, QObject const*, void**, QtPrivate::QSlotObjectBase*, Qt::ConnectionType, int const*, QMetaObj ect const*) (qobject.cpp:4789) ==26857== by 0x7B9B21B: QMetaObject::Connection QObject::connect<void (KIO::Connection::*)(), void (KIO::Slave::*)()>(QtPrivate::FunctionPointer<void (KIO::Connec tion::*)()>::Object const*, void (KIO::Connection::*)(), QtPrivate::FunctionPointer<void (KIO::Slave::*)()>::Object const*, void (KIO::Slave::*)(), Qt::ConnectionTyp e) (qobject.h:254) ==26857== by 0x7B98376: KIO::Slave::accept() (slave.cpp:174) ==26857== by 0x7B9C212: QtPrivate::FunctorCall<QtPrivate::IndexesList<>, QtPrivate::List<>, void, void (KIO::Slave::*)()>::call(void (KIO::Slave::*)(), KIO::Slave *, void**) (qobjectdefs_impl.h:134) ==26857== by 0x7B9C0F0: void QtPrivate::FunctionPointer<void (KIO::Slave::*)()>::call<QtPrivate::List<>, void>(void (KIO::Slave::*)(), KIO::Slave*, void**) (qobje ctdefs_impl.h:167) ==26857== by 0x7B9BD84: QtPrivate::QSlotObject<void (KIO::Slave::*)(), QtPrivate::List<>, void>::impl(int, QtPrivate::QSlotObjectBase*, QObject*, void**, bool*) ( qobjectdefs_impl.h:396) ==26857== by 0xD7E3BAF: QMetaObject::activate(QObject*, int, int, void**) (qobjectdefs_impl.h:376) ==26857== by 0x7C3E176: KIO::ConnectionServer::newConnection() (moc_connectionserver.cpp:127) ==26857== by 0x7C3DF93: KIO::ConnectionServer::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) (moc_connectionserver.cpp:72) ==26857== by 0xD7E3A52: QMetaObject::activate(QObject*, int, int, void**) (qobject.cpp:3771)
The old connect syntax did not leak in itself. If you see a connection leaked, it means one of the two objects (sender or receiver) isn't deleted.
Using --num-callers=50, the leak shows as:
==3437== 54,952 (32 direct, 54,920 indirect) bytes in 1 blocks are definitely lost in loss record 2,284 of 2,284 ==3437== at 0x4C2EB4F: operator new(unsigned long) (vg_replace_malloc.c:334) ==3437== by 0xD7E54A9: QObjectPrivate::addConnection(int, QObjectPrivate::Connection*) (qobject.cpp:390) ==3437== by 0xD7E64AC: QMetaObjectPrivate::connect(QObject const*, int, QMetaObject const*, QObject const*, int, QMetaObject const*, int, int*) (qobject.cpp:3319) ==3437== by 0xD7E8D95: QObject::connect(QObject const*, char const*, QObject const*, char const*, Qt::ConnectionType) (qobject.cpp:2777) ==3437== by 0x7B34899: QObject::connect(QObject const*, char const*, char const*, Qt::ConnectionType) const (qobject.h:465) ==3437== by 0x7B94057: KIO::SlaveInterface::SlaveInterface(KIO::SlaveInterfacePrivate&, QObject*) (slaveinterface.cpp:48) ==3437== by 0x7B985BC: KIO::Slave::Slave(QString const&, QObject*) (slave.cpp:216) ==3437== by 0x7B99401: KIO::Slave::createSlave(QString const&, QUrl const&, int&, QString&) (slave.cpp:474) ==3437== by 0x7BC321A: KIO::ProtoQueue::createSlave(QString const&, KIO::SimpleJob*, QUrl const&) (scheduler.cpp:542) ==3437== by 0x7BC37CA: KIO::ProtoQueue::startAJob() (scheduler.cpp:629) ==3437== by 0x7BC6FF5: KIO::ProtoQueue::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) (moc_scheduler_p.cpp:252)
The slaves registered are killed and deleted, therefore register the created slave.
This leak is gone.
As I do not know how to discard new incorrect updates... The previous incorrect one creates a crash.
Return to the good one and land.