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

QObject: attempt to fix a deadlock introduced by an earlier fix

Browse Source 
Commit 71b4d4f150bc3c904a5aceec37513ddc3cd1c150 is likely the source of
the issue. It fixed a race on disconnection, but kept the call to
disconnectNotify() (which is user code) inside the locked section. My
analysis is that by construction the sender object can't be undergoing
concurrent deletion anyway at this point. All call sites
(QObject::disconnect or the signal-slot activations but before the slot
is activated) imply that the user code that reached here cannot itself
be racing the deletion.

There may be one race condition left: if the same signal was connected
earlier to a slot via queued connection and that slot deletes the sender
asynchronously. A synchronous deletion is handled by doActivate(), so
the single-shot connection is never activated in the first place, but an
asynchronous deletion could race past that check and delete the sender
while QObjectPrivate::removeConnection is running. However, I'd call
this a mistake in user code.

[ChangeLog][QtCore][QObject] Fixed a regression from 6.3 that caused
QObject::isSignalConnected() to deadlock if called from inside
disconnectNotify().

Fixes: QTBUG-106025
Change-Id: Ic6547f8247454b47baa8fffd170fe0bdb62cfcaf
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Lars Knoll <lars@knoll.priv.no>
(cherry picked from commit 22d4c67234fd152296c3ec98fc57526356a9f62b)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
Thiago Macieira 2022-08-29 14:24:52 -03:00 committed by Qt Cherry-pick Bot
parent 7c13ad46d8
commit b46e179186
1 changed files with 11 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
src/corelib/kernel/qobject.cpp
View File

@ -5275,11 +5275,20 @@ bool QObjectPrivate::disconnect(const QObject *sender, int signal_index, const Q
/*! /*!
\internal \internal
\threadsafe \threadsafe
Thread-safety warning: this function may be called from any thread and is
thread-safe, \b{so long as the sender is not being deleted}. At the time of
this writing, this function is called from QObject::disconnect() and from
the multiple places where a single-shot connection is activated; in both
cases, the construction of the user code is already such that the sender
object cannot be undergoing deletion in another thread.
*/ */
bool QObjectPrivate::disconnect(QObjectPrivate::Connection *c) bool QObjectPrivate::disconnect(QObjectPrivate::Connection *c)
{ {
if (!c) if (!c)
return false; return false;
// double-checked locking on this pointer
QObject *receiver = c->receiver.loadRelaxed(); QObject *receiver = c->receiver.loadRelaxed();
if (!receiver) if (!receiver)
return false; return false;
@ -5300,7 +5309,6 @@ bool QObjectPrivate::disconnect(QObjectPrivate::Connection *c)
Q_ASSERT(connections); Q_ASSERT(connections);
connections->removeConnection(c); connections->removeConnection(c);
c->sender->disconnectNotify(QMetaObjectPrivate::signal(c->sender->metaObject(), c->signal_index));
// We must not hold the receiver mutex, else we risk dead-locking; we also only need the sender mutex // We must not hold the receiver mutex, else we risk dead-locking; we also only need the sender mutex
// It is however vital to hold the senderMutex before calling cleanOrphanedConnections, as otherwise // It is however vital to hold the senderMutex before calling cleanOrphanedConnections, as otherwise
// another thread might modify/delete the connection // another thread might modify/delete the connection
@ -5312,6 +5320,8 @@ bool QObjectPrivate::disconnect(QObjectPrivate::Connection *c)
locker.dismiss(); // so we dismiss the QOrderedMutexLocker locker.dismiss(); // so we dismiss the QOrderedMutexLocker
} }
// this is safe if the condition in the documentation is correct
c->sender->disconnectNotify(QMetaObjectPrivate::signal(c->sender->metaObject(), c->signal_index));
return true; return true;
} }