From c399db82a37251fd3fd4a6f47db0fdbc349afef6 Mon Sep 17 00:00:00 2001 From: Robert Osfield Date: Tue, 1 Oct 2013 09:37:56 +0000 Subject: [PATCH] From Christopher Baker, submitted by Alberto Luacas, "there is a bug report in the Ubuntu tracker that points to a bug when loading multiple VRML files in parallel. Christopher R. Baker has detected this bug and crafted a patch. In addition, libcoin has to be also built with the "--enable-threadsafe" option. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I copy here his report, extracted from (https://bugs.launchpad.net/ubuntu/+source/openscenegraph/+bug/1211993) and attach his fix. All credit is due to him: « There are three instances of a classical method-local-static multithreaded initialization bug in the Inventor plugin for OSG that trigger various memory faults when reading multiple VRML files in parallel via osgDB::readNodeFile. These bugs are of the form: static std::map myHandyMap; static bool once = true; if(once) { ...fill myHandyMap; once = false } ... use myHandyMap; To repeat: try loading multiple VRML files from multiple threads. The liklihood of the bug depends on many factors, but my application, which parallel-loads some dozens of small (<100K) VRML files on startup, triggers this problem 25% of the time or more. The attached patch (inventor-plugin-multithread.patch) rectifies this problem by: 1 - Inheriting MyHandyMap from std::map, then 2 - Moving the map initialization into the derived constructor, which 3 - Is intrinsically protected from multithread issues by g++ (and is part of the C++ standard), unless you pass -fno-threadsafe-statics, which is strongly discouraged by the man page. » " --- .../Inventor/ConvertFromInventor.cpp | 93 +++++++++---------- 1 file changed, 46 insertions(+), 47 deletions(-) diff --git a/src/osgPlugins/Inventor/ConvertFromInventor.cpp b/src/osgPlugins/Inventor/ConvertFromInventor.cpp index 3e150cc7e..e841df436 100644 --- a/src/osgPlugins/Inventor/ConvertFromInventor.cpp +++ b/src/osgPlugins/Inventor/ConvertFromInventor.cpp @@ -756,6 +756,38 @@ ConvertFromInventor::postLOD(void* data, SoCallbackAction* action, return SoCallbackAction::CONTINUE; } + +// g++ (at least) guarantees thread-safe method-local static initialization, so moving construction of these maps to exploit +class NormBindingMap : public std::map +{ + public: + NormBindingMap() + { + (*this)[SoNormalBinding::OVERALL] = deprecated_osg::Geometry::BIND_OVERALL; + (*this)[SoNormalBinding::PER_PART] = deprecated_osg::Geometry::BIND_PER_PRIMITIVE; + (*this)[SoNormalBinding::PER_PART_INDEXED] = deprecated_osg::Geometry::BIND_PER_PRIMITIVE; + (*this)[SoNormalBinding::PER_FACE] = deprecated_osg::Geometry::BIND_PER_PRIMITIVE; + (*this)[SoNormalBinding::PER_FACE_INDEXED] = deprecated_osg::Geometry::BIND_PER_PRIMITIVE; + (*this)[SoNormalBinding::PER_VERTEX] = deprecated_osg::Geometry::BIND_PER_VERTEX; + (*this)[SoNormalBinding::PER_VERTEX_INDEXED] = deprecated_osg::Geometry::BIND_PER_VERTEX; + } +}; + +class ColBindingMap : public std::map +{ + public: + ColBindingMap() + { + (*this)[SoMaterialBinding::OVERALL] = deprecated_osg::Geometry::BIND_OVERALL; + (*this)[SoMaterialBinding::PER_PART] = deprecated_osg::Geometry::BIND_PER_PRIMITIVE; + (*this)[SoMaterialBinding::PER_PART_INDEXED] = deprecated_osg::Geometry::BIND_PER_PRIMITIVE; + (*this)[SoMaterialBinding::PER_FACE] = deprecated_osg::Geometry::BIND_PER_PRIMITIVE; + (*this)[SoMaterialBinding::PER_FACE_INDEXED] = deprecated_osg::Geometry::BIND_PER_PRIMITIVE; + (*this)[SoMaterialBinding::PER_VERTEX] = deprecated_osg::Geometry::BIND_PER_VERTEX; + (*this)[SoMaterialBinding::PER_VERTEX_INDEXED] = deprecated_osg::Geometry::BIND_PER_VERTEX; + } +}; + /////////////////////////////////////////////////////////////////// SoCallbackAction::Response ConvertFromInventor::preShape(void* data, SoCallbackAction* action, @@ -769,45 +801,8 @@ ConvertFromInventor::preShape(void* data, SoCallbackAction* action, ConvertFromInventor* thisPtr = (ConvertFromInventor *) (data); // Normal and color binding map from Inventor to OSG - static std::map - normBindingMap; - static std::map - colBindingMap; - static bool firstTime = true; - if (firstTime) - { - normBindingMap[SoNormalBinding::OVERALL] - = deprecated_osg::Geometry::BIND_OVERALL; - normBindingMap[SoNormalBinding::PER_PART] - = deprecated_osg::Geometry::BIND_PER_PRIMITIVE; - normBindingMap[SoNormalBinding::PER_PART_INDEXED] - = deprecated_osg::Geometry::BIND_PER_PRIMITIVE; - normBindingMap[SoNormalBinding::PER_FACE] - = deprecated_osg::Geometry::BIND_PER_PRIMITIVE; - normBindingMap[SoNormalBinding::PER_FACE_INDEXED] - = deprecated_osg::Geometry::BIND_PER_PRIMITIVE; - normBindingMap[SoNormalBinding::PER_VERTEX] - = deprecated_osg::Geometry::BIND_PER_VERTEX; - normBindingMap[SoNormalBinding::PER_VERTEX_INDEXED] - = deprecated_osg::Geometry::BIND_PER_VERTEX; - - colBindingMap[SoMaterialBinding::OVERALL] - = deprecated_osg::Geometry::BIND_OVERALL; - colBindingMap[SoMaterialBinding::PER_PART] - = deprecated_osg::Geometry::BIND_PER_PRIMITIVE; - colBindingMap[SoMaterialBinding::PER_PART_INDEXED] - = deprecated_osg::Geometry::BIND_PER_PRIMITIVE; - colBindingMap[SoMaterialBinding::PER_FACE] - = deprecated_osg::Geometry::BIND_PER_PRIMITIVE; - colBindingMap[SoMaterialBinding::PER_FACE_INDEXED] - = deprecated_osg::Geometry::BIND_PER_PRIMITIVE; - colBindingMap[SoMaterialBinding::PER_VERTEX] - = deprecated_osg::Geometry::BIND_PER_VERTEX; - colBindingMap[SoMaterialBinding::PER_VERTEX_INDEXED] - = deprecated_osg::Geometry::BIND_PER_VERTEX; - - firstTime = false; - } + static NormBindingMap normBindingMap; + static ColBindingMap colBindingMap; // Get normal and color binding if (node->isOfType(SoVertexShape::getClassTypeId())) @@ -1877,6 +1872,17 @@ update: The mentioned bug is probably just for very old NVidia drivers (commit t return stateSet; } + +class TexWrapMap : public std::map +{ + public: + TexWrapMap() + { + (*this)[SoTexture2::CLAMP] = osg::Texture2D::CLAMP; + (*this)[SoTexture2::REPEAT] = osg::Texture2D::REPEAT; + } +}; + //////////////////////////////////////////////////////////////////// osg::Texture2D* ConvertFromInventor::convertIVTexToOSGTex(const SoNode* soNode, @@ -1946,14 +1952,7 @@ ConvertFromInventor::convertIVTexToOSGTex(const SoNode* soNode, // Set name osgTex->setName(soNode->getName().getString()); - static std::map texWrapMap; - static bool firstTime = true; - if (firstTime) - { - texWrapMap[SoTexture2::CLAMP] = osg::Texture2D::CLAMP; - texWrapMap[SoTexture2::REPEAT] = osg::Texture2D::REPEAT; - firstTime = false; - } + static TexWrapMap texWrapMap; // Set texture wrap mode #ifdef __COIN__