From e9eddfd85628f0ec672895652c67443caa160b7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20Nordheim?= Date: Fri, 16 Aug 2019 15:49:42 +0200 Subject: [PATCH] Fix rare double-free in QObject machinery As exposed by tst_QObjectRace::destroyRace we would sometimes end up with a double-free when destroying a QSlotObject in multi-threaded scenarios. One free would be done in ~QObject as the receiver was being destroyed while the other free was done when deleting a QMetaCallEvent object after we realized it was not needed because the receiver was destroyed. Since we can be in a separate thread from the receiver we should lock before referencing the connection object. Amends b7d073e9905bf9812ba96cecdcf6871a95517d30. Change-Id: Icb53862dc880ae9a4e5581a1a9ee693573f7d9c7 Reviewed-by: Volker Hilsheimer --- src/corelib/kernel/qobject.cpp | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp index 82fe5467155..4a26e9cdc04 100644 --- a/src/corelib/kernel/qobject.cpp +++ b/src/corelib/kernel/qobject.cpp @@ -3728,6 +3728,15 @@ static void queued_activate(QObject *sender, int signal, QObjectPrivate::Connect while (argumentTypes[nargs-1]) ++nargs; + QBasicMutexLocker locker(signalSlotLock(c->receiver.loadRelaxed())); + if (!c->receiver.loadRelaxed()) { + // the connection has been disconnected before we got the lock + return; + } + if (c->isSlotObject) + c->slotObj->ref(); + locker.unlock(); + QMetaCallEvent *ev = c->isSlotObject ? new QMetaCallEvent(c->slotObj, sender, signal, nargs) : new QMetaCallEvent(c->method_offset, c->method_relative, c->callFunction, sender, signal, nargs); @@ -3746,9 +3755,11 @@ static void queued_activate(QObject *sender, int signal, QObjectPrivate::Connect args[n] = QMetaType::create(types[n], argv[n]); } - QBasicMutexLocker locker(signalSlotLock(c->receiver.loadRelaxed())); + locker.relock(); + if (c->isSlotObject) + c->slotObj->destroyIfLastRef(); if (!c->receiver.loadRelaxed()) { - // the connection has been disconnected before we got the lock + // the connection has been disconnected while we were unlocked locker.unlock(); delete ev; return;