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
Change-Id: I9ef857efa609b4d16ee21063ccccd316e119576b
Reviewed-by: Sona Kurazyan <sona.kurazyan@qt.io>
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
(cherry picked from commit 42e13b7c61693ca95e119106d4f6dbd2bcf2308d)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

1 files changed, 3 insertions, 0 deletions

diff --git a/src/gui/util/qdesktopservices.cpp b/src/gui/util/qdesktopservices.cpp
index 8f1b035483..be63b3768a 100644
--- a/src/gui/util/qdesktopservices.cpp
+++ b/src/gui/util/qdesktopservices.cpp
@@ -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) {