ShaderData: Atomically generate property values

Ensure same lock is used when generating property values for ShaderData.
Not doing so could cause a race condition if two different threads
checked if ShaderData contained property value for a given block at the
same time.

Also removes unused method to avoid similary issues in the future.

Fixes: QTBUG-111427
Pick-to: 6.7 6.6
Change-Id: Iadf88aeb7a440bcfd46f2a8e164403fa7d6e1713
Reviewed-by: Paul Lemire <paul.lemire@kdab.com>

3 files changed, 11 insertions, 12 deletions

diff --git a/src/render/jobs/uniformblockbuilder.cpp b/src/render/jobs/uniformblockbuilder.cpp
index 462d66176..ec6c72199 100644
--- a/src/render/jobs/uniformblockbuilder.cpp
+++ b/src/render/jobs/uniformblockbuilder.cpp
@@ -97,8 +97,7 @@ void UniformBlockValueBuilder::buildActiveUniformNameValueMapStructHelper(Shader
 
     // Retrieve set of {NameId -> PropertyValue} for Block
     const int fullBlockNameId = StringToInt::lookupId(fullBlockName);
-    if (!rShaderData->hasPropertyValuesForBlock(fullBlockNameId))
-        rShaderData->generatePropertyValuesForBlock(fullBlockName);
+    rShaderData->generatePropertyValuesForBlockIfNeeded(fullBlockName);
     const ShaderData::PropertyValuesForBlock &propertiesForBlock = rShaderData->propertyValuesForBlock(fullBlockNameId);
 
     for (const auto &nameIdPropertyPair : propertiesForBlock) {
diff --git a/src/render/materialsystem/shaderdata.cpp b/src/render/materialsystem/shaderdata.cpp
index b1e8e0f00..ad6dbfff7 100644
--- a/src/render/materialsystem/shaderdata.cpp
+++ b/src/render/materialsystem/shaderdata.cpp
@@ -122,20 +122,22 @@ void ShaderData::syncFromFrontEnd(const QNode *frontEnd, bool firstTime)
     }
 }
 
-bool ShaderData::hasPropertyValuesForBlock(int blockName) const
-{
-    std::shared_lock readLocker(m_lock);
-    return m_blockNameToPropertyValues.find(blockName) != m_blockNameToPropertyValues.cend();
-}
-
 const ShaderData::PropertyValuesForBlock &ShaderData::propertyValuesForBlock(int blockName) const
 {
     std::shared_lock readLocker(m_lock);
     return m_blockNameToPropertyValues.at(blockName);
 }
 
-void ShaderData::generatePropertyValuesForBlock(const QString &fullBlockName)
+void ShaderData::generatePropertyValuesForBlockIfNeeded(const QString &fullBlockName)
 {
+    const int fullBlockNameId = StringToInt::lookupId(fullBlockName);
+
+    std::unique_lock readWriteLocker(m_lock);
+    const bool hasPropertyValuesForBlock = m_blockNameToPropertyValues.find(fullBlockNameId) != m_blockNameToPropertyValues.cend();
+    if (hasPropertyValuesForBlock) {
+        return;
+    }
+
     const QHash<QString, ShaderData::PropertyValue> &props = properties();
 
     ShaderData::PropertyValuesForBlock valueBlock;
@@ -162,7 +164,6 @@ void ShaderData::generatePropertyValuesForBlock(const QString &fullBlockName)
         ++it;
     }
 
-    std::unique_lock writeLocker(m_lock);
     m_blockNameToPropertyValues[StringToInt::lookupId(fullBlockName)] = std::move(valueBlock);
 }
 
diff --git a/src/render/materialsystem/shaderdata_p.h b/src/render/materialsystem/shaderdata_p.h
index 440819e38..cf29742b6 100644
--- a/src/render/materialsystem/shaderdata_p.h
+++ b/src/render/materialsystem/shaderdata_p.h
@@ -74,9 +74,8 @@ public:
     // Block.Property nameId, property nameId, PropertyValue *
     using PropertyValuesForBlock = std::vector<std::tuple<int, int, const PropertyValue *>>;
 
-    bool hasPropertyValuesForBlock(int blockNameId) const;
     const PropertyValuesForBlock &propertyValuesForBlock(int blockNameId) const;
-    void generatePropertyValuesForBlock(const QString &blockName);
+    void generatePropertyValuesForBlockIfNeeded(const QString &blockName);
 
 protected:
     PropertyReaderInterfacePtr m_propertyReader;