Crash fix: use cache to find materials.

Add a thread-safe cache of the currently valid materials, to avoid
the osgDB thread making repeated SGCondition evaluations in conflict
with the main thread.

(Requires an accompanying FG change)

simgear/scene/material/matlib.cxx

@@ -40,6 +40,8 @@
 #include <simgear/props/props_io.hxx>
 #include <simgear/props/condition.hxx>
 #include <simgear/scene/tgdb/userdata.hxx>
+#include <simgear/threads/SGThread.hxx>
+#include <simgear/threads/SGGuard.hxx>
 
 #include "mat.hxx"
 
@@ -49,8 +51,17 @@
 
 using std::string;
 
+
+class SGMaterialLib::MatLibPrivate
+{
+public:
+    SGMutex mutex;
+};
+
 // Constructor
-SGMaterialLib::SGMaterialLib ( void ) {
+SGMaterialLib::SGMaterialLib ( void ) :
+    d(new MatLibPrivate)
+{
 }
 
 // Load a library of material properties
@@ -96,15 +107,16 @@ bool SGMaterialLib::load( const string &fg_root, const string& mpath,
 }
 
 // find a material record by material name
-SGMaterial *SGMaterialLib::find( const string& material ) {
+SGMaterial *SGMaterialLib::find( const string& material ) const
+{
     SGMaterial *result = NULL;
-    material_map_iterator it = matlib.find( material );
+    const_material_map_iterator it = matlib.find( material );
     if ( it != end() ) {            
         // We now have a list of materials that match this
         // name. Find the first one that either doesn't have
         // a condition, or has a condition that evaluates
         // to true.
-        material_list_iterator iter = it->second.begin();        
+        material_list::const_iterator iter = it->second.begin();
         while (iter != it->second.end()) {            
             result = *iter;
             if (result->valid()) {
@@ -117,6 +129,31 @@ SGMaterial *SGMaterialLib::find( const string& material ) {
     return NULL;
 }
 
+void SGMaterialLib::refreshActiveMaterials()
+{
+    active_material_cache newCache;
+    material_map_iterator it = matlib.begin();
+    for (; it != matlib.end(); ++it) {
+        newCache[it->first] = find(it->first);
+    }
+    
+    // use this approach to minimise the time we're holding the lock
+    // lock on the mutex (and hence, would block findCached calls)
+    SGGuard<SGMutex> g(d->mutex);
+    active_cache = newCache;
+}
+
+SGMaterial *SGMaterialLib::findCached( const string& material ) const
+{
+    SGGuard<SGMutex> g(d->mutex);
+    
+    active_material_cache::const_iterator it = active_cache.find(material);
+    if (it == active_cache.end())
+        return NULL;
+    
+    return it->second;
+}
+
 // Destructor
 SGMaterialLib::~SGMaterialLib ( void ) {
     SG_LOG( SG_GENERAL, SG_INFO, "SGMaterialLib::~SGMaterialLib() size=" << matlib.size());

simgear/scene/material/matlib.hxx

@@ -29,6 +29,7 @@
 
 #include <simgear/structure/SGSharedPtr.hxx>
 
+#include <memory>
 #include <string>		// Standard C++ string library
 #include <map>			// STL associative "array"
 #include <vector>		// STL "array"
@@ -43,16 +44,22 @@ namespace osg { class Geode; }
 class SGMaterialLib {
 
 private:
-
+    class MatLibPrivate;
+    std::auto_ptr<MatLibPrivate> d;
+    
     // associative array of materials
     typedef std::vector< SGSharedPtr<SGMaterial> > material_list;    
     typedef material_list::iterator material_list_iterator;
+    
     typedef std::map < std::string,  material_list> material_map;
     typedef material_map::iterator material_map_iterator;
     typedef material_map::const_iterator const_material_map_iterator;
 
     material_map matlib;
-
+    
+    typedef std::map < std::string, SGSharedPtr<SGMaterial> > active_material_cache;
+    active_material_cache active_cache;
+    
 public:
 
     // Constructor
@@ -62,8 +69,24 @@ public:
     bool load( const std::string &fg_root, const std::string& mpath,
             SGPropertyNode *prop_root );
     // find a material record by material name
-    SGMaterial *find( const std::string& material );
+    SGMaterial *find( const std::string& material ) const;
 
+    /**
+     * Material lookup involves evaluation of SGConditions to determine which
+     * possible material (by season, region, etc) is valid. This involves
+     * vproperty tree queries, so repeated calls to find() can cause
+     * race conditions when called from the osgDB pager thread. (especially
+     * during startup)
+     *
+     * To fix this, and also avoid repeated re-evaluation of the material
+     * conditions, we provide a version which uses a cached, threadsafe table
+     * of the currently valid materials. The main thread calls the refresh
+     * method below to evaluate the valid materials, and findCached can be
+     * safely called from other threads with no access to unprotected state.
+     */
+    SGMaterial *findCached( const std::string& material ) const;
+    void refreshActiveMaterials();
+    
     material_map_iterator begin() { return matlib.begin(); }
     const_material_map_iterator begin() const { return matlib.begin(); }
 

simgear/scene/tgdb/SGOceanTile.cxx

@@ -282,7 +282,7 @@ osg::Node* SGOceanTile(const SGBucket& b, SGMaterialLib *matlib, int latPoints,
     double tex_width = 1000.0;
   
     // find Ocean material in the properties list
-    SGMaterial *mat = matlib->find( "Ocean" );
+    SGMaterial *mat = matlib->findCached( "Ocean" );
     if ( mat != NULL ) {
         // set the texture width and height values for this
         // material

simgear/scene/tgdb/obj.cxx

@@ -146,7 +146,7 @@ public:
       std::string materialName = obj.get_pt_materials()[grp];
       SGMaterial* material = 0;
       if (matlib)
-          material = matlib->find(materialName);
+          material = matlib->findCached(materialName);
       SGVec4f color = getMaterialLightColor(material);
 
       if (3 <= materialName.size() && materialName.substr(0, 3) != "RWY") {
@@ -326,7 +326,7 @@ public:
   {
     if (!matlib)
       return SGVec2f(1, 1);
-    SGMaterial* material = matlib->find(name);
+    SGMaterial* material = matlib->findCached(name);
     if (!material)
       return SGVec2f(1, 1);
 
@@ -400,7 +400,7 @@ public:
       osg::Geometry* geometry = i->second.buildGeometry();
       SGMaterial *mat = 0;
       if (matlib)
-        mat = matlib->find(i->first);
+        mat = matlib->findCached(i->first);
       eg = new EffectGeode;
       eg->setName("EffectGeode");
       if (mat)
@@ -427,7 +427,7 @@ public:
     mt_init(&seed, unsigned(123));
 
     for (i = materialTriangleMap.begin(); i != materialTriangleMap.end(); ++i) {
-      SGMaterial *mat = matlib->find(i->first);
+      SGMaterial *mat = matlib->findCached(i->first);
       if (!mat)
         continue;
 
@@ -482,7 +482,7 @@ public:
     mt_init(&seed, unsigned(123));
 
     for (i = materialTriangleMap.begin(); i != materialTriangleMap.end(); ++i) {
-      SGMaterial *mat = matlib->find(i->first);
+      SGMaterial *mat = matlib->findCached(i->first);
       SGTexturedTriangleBin triangleBin = i->second;
 
       if (!mat)
@@ -799,7 +799,7 @@ public:
     mt_init(&seed, unsigned(586));
 
     for (i = materialTriangleMap.begin(); i != materialTriangleMap.end(); ++i) {
-      SGMaterial *mat = matlib->find(i->first);
+      SGMaterial *mat = matlib->findCached(i->first);
       if (!mat)
         continue;
 
@@ -955,13 +955,13 @@ public:
         SGVec4f red(1, 0, 0, 1);
         SGMaterial* mat = 0;
         if (matlib)
-          mat = matlib->find("RWY_RED_LIGHTS");
+          mat = matlib->findCached("RWY_RED_LIGHTS");
         if (mat)
           red = mat->get_light_color();
         SGVec4f white(1, 1, 1, 1);
         mat = 0;
         if (matlib)
-          mat = matlib->find("RWY_WHITE_LIGHTS");
+          mat = matlib->findCached("RWY_WHITE_LIGHTS");
         if (mat)
           white = mat->get_light_color();
         SGDirectionalLightListBin::const_iterator i;