[webkit-changes] [189580] trunk/Source/WebCore

[commit-queue]

Title: [189580] trunk/Source/WebCore

Revision

189580

Author

commit-qu...@webkit.org

Date

2015-09-10 11:47:56 -0700 (Thu, 10 Sep 2015)

Log Message

Static variables in GraphicsContext3DOpenGLCommon should be avoided because of the race condition
https://bugs.webkit.org/show_bug.cgi?id=148957

Patch by Jinyoung Hur <hur....@navercorp.com> on 2015-09-10
Reviewed by Dean Jackson.

There is no guarantee that only one thread calls GraphicsContext3D::compileShader() at a time so it would be
better to use a thread local storage variable rather than use a static variable.

No new tests. No behavioural changes.

* platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:
(WebCore::getCurrentNameHashMapForShader):
(WebCore::setCurrentNameHashMapForShader):
(WebCore::nameHashForShader):
(WebCore::GraphicsContext3D::compileShader):
(WebCore::GraphicsContext3D::mappedSymbolName):

Modified Paths

• trunk/Source/WebCore/ChangeLog
• trunk/Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp

Diff

Modified: trunk/Source/WebCore/ChangeLog (189579 => 189580)

--- trunk/Source/WebCore/ChangeLog	2015-09-10 18:15:45 UTC (rev 189579)
+++ trunk/Source/WebCore/ChangeLog	2015-09-10 18:47:56 UTC (rev 189580)
@@ -1,3 +1,22 @@
+2015-09-10  Jinyoung Hur  <hur....@navercorp.com>
+
+        Static variables in GraphicsContext3DOpenGLCommon should be avoided because of the race condition
+        https://bugs.webkit.org/show_bug.cgi?id=148957
+
+        Reviewed by Dean Jackson.
+
+        There is no guarantee that only one thread calls GraphicsContext3D::compileShader() at a time so it would be 
+        better to use a thread local storage variable rather than use a static variable.
+
+        No new tests. No behavioural changes.
+
+        * platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:
+        (WebCore::getCurrentNameHashMapForShader):
+        (WebCore::setCurrentNameHashMapForShader):
+        (WebCore::nameHashForShader):
+        (WebCore::GraphicsContext3D::compileShader):
+        (WebCore::GraphicsContext3D::mappedSymbolName):
+
 2015-09-10  Chris Dumez  <cdu...@apple.com>
 
         Node.appendChild(null) / replaceChild(null, null) / removeChild(null) / insertBefore(null, ref) should throw a TypeError

Modified: trunk/Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp (189579 => 189580)

--- trunk/Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp	2015-09-10 18:15:45 UTC (rev 189579)
+++ trunk/Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp	2015-09-10 18:47:56 UTC (rev 189580)
@@ -79,8 +79,22 @@
 
 namespace WebCore {
 
-static ShaderNameHash* currentNameHashMapForShader = nullptr;
+static ThreadSpecific<ShaderNameHash*>& getCurrentNameHashMapForShader()
+{
+    static std::once_flag onceFlag;
+    static ThreadSpecific<ShaderNameHash*>* sharedNameHash;
+    std::call_once(onceFlag, [] {
+        sharedNameHash = new ThreadSpecific<ShaderNameHash*>;
+    });
 
+    return *sharedNameHash;
+}
+
+static void setCurrentNameHashMapForShader(ShaderNameHash* shaderNameHash)
+{
+    *getCurrentNameHashMapForShader() = shaderNameHash;
+}
+
 // Hash function used by the ANGLE translator/compiler to do
 // symbol name mangling. Since this is a static method, before
 // calling compileShader we set currentNameHashMapForShader
@@ -94,11 +108,10 @@
     CString nameAsCString = CString(name);
 
     // Look up name in our local map.
-    if (currentNameHashMapForShader) {
-        ShaderNameHash::iterator result = currentNameHashMapForShader->find(nameAsCString);
-        if (result != currentNameHashMapForShader->end())
-            return result->value;
-    }
+    ShaderNameHash*& currentNameHashMapForShader = *getCurrentNameHashMapForShader();
+    ShaderNameHash::iterator findResult = currentNameHashMapForShader->find(nameAsCString);
+    if (findResult != currentNameHashMapForShader->end())
+        return findResult->value;
 
     unsigned hashValue = nameAsCString.hash();
 
@@ -568,14 +581,14 @@
 
     if (!nameHashMapForShaders)
         nameHashMapForShaders = std::make_unique<ShaderNameHash>();
-    currentNameHashMapForShader = nameHashMapForShaders.get();
+    setCurrentNameHashMapForShader(nameHashMapForShaders.get());
     m_compiler.setResources(ANGLEResources);
 
     String translatedShaderSource = m_extensions->getTranslatedShaderSourceANGLE(shader);
 
     ANGLEResources.HashFunction = previousHashFunction;
     m_compiler.setResources(ANGLEResources);
-    currentNameHashMapForShader = nullptr;
+    setCurrentNameHashMapForShader(nullptr);
 
     if (!translatedShaderSource.length())
         return;
@@ -877,11 +890,11 @@
         // and aren't even required to be used in any shader program.
         if (!nameHashMapForShaders)
             nameHashMapForShaders = std::make_unique<ShaderNameHash>();
-        currentNameHashMapForShader = nameHashMapForShaders.get();
+        setCurrentNameHashMapForShader(nameHashMapForShaders.get());
 
         String generatedName = generateHashedName(name);
 
-        currentNameHashMapForShader = nullptr;
+        setCurrentNameHashMapForShader(nullptr);
 
         m_possiblyUnusedAttributeMap.set(generatedName, name);
 

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes