QDesktopServices: fix UB (data race on handlers)

The handlerDestroyed() function is connected to the handler QObject's
destroyed() signal and removes all entries from the registry for which
the destroyed object was listed as the handler.

While handlerDestroyed() is always executed in the context of the
thread owning QOpenUrlHandlerRegistry-as-a-QObject, the documentation
explicitly states that "the handler will always be called from within
the same thread that calls QDesktopServices::openUrl()", implying that
calling openUrl() from a non-GUI thread is supported. The presence of
the mutex also indicates that this should work.

But then the unprotected access to the handlers variable in
handlerDestroyed() is a data race, because nothing prevents a handler
object from being destroyed concurrent to an openUrl() call.

Fix by locking the mutex.

Fixes: QTBUG-100777
Pick-to: 6.3 6.2 5.15
Change-Id: I9ef857efa609b4d16ee21063ccccd316e119576b
Reviewed-by: Sona Kurazyan <sona.kurazyan@qt.io>
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
This commit is contained in:
Marc Mutz 2022-02-11 09:12:49 +01:00
parent 4fd4082c3a
commit 42e13b7c61

View File

@ -54,6 +54,8 @@
#include <qpa/qplatformintegration.h>
#include <qdir.h>
#include <QtCore/private/qlocking_p.h>
QT_BEGIN_NAMESPACE
class QOpenUrlHandlerRegistry : public QObject
@ -81,6 +83,7 @@ Q_GLOBAL_STATIC(QOpenUrlHandlerRegistry, handlerRegistry)
void QOpenUrlHandlerRegistry::handlerDestroyed(QObject *handler)
{
const auto lock = qt_scoped_lock(mutex);
HandlerHash::Iterator it = handlers.begin();
while (it != handlers.end()) {
if (it->receiver == handler) {