From dc9cb0562afce3d794acef07ea35ad87cffa2464 Mon Sep 17 00:00:00 2001
From: Jarek Kobus <jaroslaw.kobus@qt.io>
Date: Mon, 7 Nov 2022 12:19:51 +0100
Subject: [PATCH] Utils::Id: Make access to internal cache thread safe

Secure all accesses to internal cache with QReadWriteLock.
Move firstUnusedId into secured scope, too.

Fixes: QTCREATORBUG-28415
Change-Id: I99d23213ec169b2b74748f54c98b834f88ab6a3d
Reviewed-by: Eike Ziller <eike.ziller@qt.io>
Reviewed-by: hjk <hjk@qt.io>
---
 src/libs/utils/id.cpp | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/src/libs/utils/id.cpp b/src/libs/utils/id.cpp
index 13f868686d8..7f31b8f926f 100644
--- a/src/libs/utils/id.cpp
+++ b/src/libs/utils/id.cpp
@@ -10,6 +10,7 @@
 #include <QDataStream>
 #include <QDebug>
 #include <QHash>
+#include <QReadWriteLock>
 #include <QVariant>
 
 namespace Utils {
@@ -76,21 +77,31 @@ struct IdCache : public QHash<StringHolder, quintptr>
 #endif
 };
 
-
 static QHash<quintptr, StringHolder> stringFromId;
 static IdCache idFromString;
+static QReadWriteLock s_cacheMutex;
 
 static quintptr theId(const char *str, int n = 0)
 {
-    static quintptr firstUnusedId = 10 * 1000 * 1000;
     QTC_ASSERT(str && *str, return 0);
     StringHolder sh(str, n);
-    int res = idFromString.value(sh, 0);
+    int res = 0;
+    {
+        QReadLocker lock(&s_cacheMutex); // Try quick read locker first
+        res = idFromString.value(sh, 0);
+    }
     if (res == 0) {
-        res = firstUnusedId++;
-        sh.str = qstrdup(sh.str);
-        idFromString[sh] = res;
-        stringFromId[res] = sh;
+        QWriteLocker lock(&s_cacheMutex);
+        res = idFromString.value(sh, 0); // Some other thread could have added it to the cache
+                                         // in meantime, after read lock was released and before
+                                         // write lock was acquired. Re-read it again.
+        if (res == 0) {
+            static quintptr firstUnusedId = 10 * 1000 * 1000;
+            res = firstUnusedId++;
+            sh.str = qstrdup(sh.str);
+            idFromString[sh] = res;
+            stringFromId[res] = sh;
+        }
     }
     return res;
 }
@@ -127,6 +138,7 @@ Id::Id(const char *name)
 
 QByteArray Id::name() const
 {
+    QReadLocker lock(&s_cacheMutex);
     return stringFromId.value(m_id).str;
 }
 
@@ -142,6 +154,7 @@ QByteArray Id::name() const
 
 QString Id::toString() const
 {
+    QReadLocker lock(&s_cacheMutex);
     return QString::fromUtf8(stringFromId.value(m_id).str);
 }
 
@@ -188,6 +201,7 @@ Id Id::fromName(const QByteArray &name)
 
 QVariant Id::toSetting() const
 {
+    QReadLocker lock(&s_cacheMutex);
     return QVariant(QString::fromUtf8(stringFromId.value(m_id).str));
 }
 
@@ -280,6 +294,7 @@ Id Id::withPrefix(const char *prefix) const
 
 bool Id::operator==(const char *name) const
 {
+    QReadLocker lock(&s_cacheMutex);
     const char *string = stringFromId.value(m_id).str;
     if (string && name)
         return strcmp(string, name) == 0;
@@ -290,6 +305,7 @@ bool Id::operator==(const char *name) const
 // For debugging purposes
 QTCREATOR_UTILS_EXPORT const char *nameForId(quintptr id)
 {
+    QReadLocker lock(&s_cacheMutex);
     return stringFromId.value(id).str;
 }