From 297e5095762be2fc0fe7995f7115e2e58edb09ae Mon Sep 17 00:00:00 2001
From: James Turner <zakalawe@mac.com>
Date: Fri, 14 Aug 2020 16:48:51 +0100
Subject: [PATCH] Avoid a race in MatModels loading

Add a mutex to ensure an SGMatModels only loads its models once. Caught
be ASan, hurrah.
---
 simgear/scene/material/matmodel.cxx | 64 +++++++++++++++--------------
 simgear/scene/material/matmodel.hxx |  2 +
 2 files changed, 35 insertions(+), 31 deletions(-)

diff --git a/simgear/scene/material/matmodel.cxx b/simgear/scene/material/matmodel.cxx
index 5cbcd242..c45446e0 100644
--- a/simgear/scene/material/matmodel.cxx
+++ b/simgear/scene/material/matmodel.cxx
@@ -103,37 +103,39 @@ SGMatModel::get_model_count( SGPropertyNode *prop_root )
 inline void
 SGMatModel::load_models( SGPropertyNode *prop_root )
 {
-				// Load model only on demand
-  if (!_models_loaded) {
-    for (unsigned int i = 0; i < _paths.size(); i++) {
-      osg::Node *entity = SGModelLib::loadModel(_paths[i], prop_root);
-      if (entity != 0) {
-        // FIXME: this stuff can be handled
-        // in the XML wrapper as well (at least,
-        // the billboarding should be handled
-        // there).
-        
-        if (_heading_type == HEADING_BILLBOARD) {
-          // if the model is a billboard, it is likely :
-          // 1. a branch with only leaves,
-          // 2. a tree or a non rectangular shape faked by transparency
-          // We add alpha clamp then
-          osg::StateSet* stateSet = entity->getOrCreateStateSet();
-          osg::AlphaFunc* alphaFunc =
-            new osg::AlphaFunc(osg::AlphaFunc::GREATER, 0.01f);
-          stateSet->setAttributeAndModes(alphaFunc,
-                                         osg::StateAttribute::OVERRIDE);
-          stateSet->setRenderingHint(osg::StateSet::TRANSPARENT_BIN);
-        } 
-        
-        _models.push_back(entity);
-        
-      } else {
-        SG_LOG(SG_INPUT, SG_ALERT, "Failed to load object " << _paths[i]);
-        // Ensure the vector contains something, otherwise get_random_model below fails
-        _models.push_back(new osg::Node());
-      }
-    }
+    std::lock_guard<std::mutex> g(_loadMutex);
+
+    // Load model only on demand
+    if (!_models_loaded) {
+        for (unsigned int i = 0; i < _paths.size(); i++) {
+            osg::Node* entity = SGModelLib::loadModel(_paths[i], prop_root);
+            if (entity != 0) {
+                // FIXME: this stuff can be handled
+                // in the XML wrapper as well (at least,
+                // the billboarding should be handled
+                // there).
+
+                if (_heading_type == HEADING_BILLBOARD) {
+                    // if the model is a billboard, it is likely :
+                    // 1. a branch with only leaves,
+                    // 2. a tree or a non rectangular shape faked by transparency
+                    // We add alpha clamp then
+                    osg::StateSet* stateSet = entity->getOrCreateStateSet();
+                    osg::AlphaFunc* alphaFunc =
+                        new osg::AlphaFunc(osg::AlphaFunc::GREATER, 0.01f);
+                    stateSet->setAttributeAndModes(alphaFunc,
+                                                   osg::StateAttribute::OVERRIDE);
+                    stateSet->setRenderingHint(osg::StateSet::TRANSPARENT_BIN);
+                }
+
+                _models.push_back(entity);
+
+            } else {
+                SG_LOG(SG_INPUT, SG_ALERT, "Failed to load object " << _paths[i]);
+                // Ensure the vector contains something, otherwise get_random_model below fails
+                _models.push_back(new osg::Node());
+            }
+        }
   }
   _models_loaded = true;
 }
diff --git a/simgear/scene/material/matmodel.hxx b/simgear/scene/material/matmodel.hxx
index 4ac45db3..33fa4134 100644
--- a/simgear/scene/material/matmodel.hxx
+++ b/simgear/scene/material/matmodel.hxx
@@ -149,6 +149,8 @@ private:
     double _spacing_m;
     double _range_m;
     HeadingType _heading_type;
+
+    std::mutex _loadMutex;
 };