Properly initialize local static pointers and collections

Static variables should be initalized once instead of initializing them with
nullptr and then doing actual initialization behind if condition. Otherwise a
race condition may happen leading to undefined behaviour.

apps/openmw/mwmechanics/mechanicsmanagerimp.cpp

@@ -846,14 +846,12 @@ namespace MWMechanics
         mAI = true;
     }
 
-    bool MechanicsManager::isBoundItem(const MWWorld::Ptr& item)
+    namespace
     {
-        static std::set<ESM::RefId> boundItemIDCache;
+        std::set<ESM::RefId> makeBoundItemIdCache()
-
-        // If this is empty then we haven't executed the GMST cache logic yet; or there isn't any sMagicBound* GMST's
-        // for some reason
-        if (boundItemIDCache.empty())
         {
+            std::set<ESM::RefId> boundItemIDCache;
+
             // Build a list of known bound item ID's
             const MWWorld::Store<ESM::GameSetting>& gameSettings
                 = MWBase::Environment::get().getESMStore()->get<ESM::GameSetting>();
@@ -870,15 +868,16 @@ namespace MWMechanics
 
                 boundItemIDCache.insert(ESM::RefId::stringRefId(currentGMSTValue));
             }
+
+            return boundItemIDCache;
         }
+    }
 
-        // Perform bound item check and assign the Flag_Bound bit if it passes
+    bool MechanicsManager::isBoundItem(const MWWorld::Ptr& item)
-        const ESM::RefId& tempItemID = item.getCellRef().getRefId();
+    {
+        static const std::set<ESM::RefId> boundItemIdCache = makeBoundItemIdCache();
 
-        if (boundItemIDCache.count(tempItemID) != 0)
+        return boundItemIdCache.find(item.getCellRef().getRefId()) != boundItemIdCache.end();
-            return true;
-
-        return false;
     }
 
     bool MechanicsManager::isAllowedToUse(const MWWorld::Ptr& ptr, const MWWorld::Ptr& target, MWWorld::Ptr& victim)

apps/openmw/mwrender/animation.cpp

@@ -388,17 +388,20 @@ namespace
         std::string_view mEffectId;
     };
 
-    osg::ref_ptr<osg::LightModel> getVFXLightModelInstance()
+    namespace
     {
-        static osg::ref_ptr<osg::LightModel> lightModel = nullptr;
+        osg::ref_ptr<osg::LightModel> makeVFXLightModelInstance()
-
-        if (!lightModel)
         {
-            lightModel = new osg::LightModel;
+            osg::ref_ptr<osg::LightModel> lightModel = new osg::LightModel;
             lightModel->setAmbientIntensity({ 1, 1, 1, 1 });
+            return lightModel;
         }
 
-        return lightModel;
+        const osg::ref_ptr<osg::LightModel>& getVFXLightModelInstance()
+        {
+            static const osg::ref_ptr<osg::LightModel> lightModel = makeVFXLightModelInstance();
+            return lightModel;
+        }
     }
 
     void assignBoneBlendCallbackRecursive(MWRender::BoneAnimBlendController* controller, osg::Node* parent, bool isRoot)

apps/openmw/mwworld/scene.cpp

@@ -451,8 +451,7 @@ namespace MWWorld
             }
             else if (!ESM::isEsm4Ext(worldspace))
             {
-                static std::vector<float> defaultHeight;
+                static const std::vector<float> defaultHeight(verts * verts, ESM::Land::DEFAULT_HEIGHT);
-                defaultHeight.resize(verts * verts, ESM::Land::DEFAULT_HEIGHT);
                 mPhysics->addHeightField(defaultHeight.data(), cellX, cellY, worldsize, verts,
                     ESM::Land::DEFAULT_HEIGHT, ESM::Land::DEFAULT_HEIGHT, land.get());
             }

components/esm3/loadmgef.cpp

@@ -132,53 +132,61 @@ namespace ESM
         esm.writeHNOString("DESC", mDescription);
     }
 
+    namespace
+    {
+        std::map<short, short> makeEffectsMap()
+        {
+            std::map<short, short> effects;
+
+            effects[MagicEffect::Effects::DisintegrateArmor] = MagicEffect::Effects::Sanctuary;
+            effects[MagicEffect::Effects::DisintegrateWeapon] = MagicEffect::Effects::Sanctuary;
+
+            for (int i = MagicEffect::Effects::DrainAttribute; i <= MagicEffect::Effects::DamageSkill; ++i)
+                effects[i] = MagicEffect::Effects::ResistMagicka;
+            for (int i = MagicEffect::Effects::AbsorbAttribute; i <= MagicEffect::Effects::AbsorbSkill; ++i)
+                effects[i] = MagicEffect::Effects::ResistMagicka;
+            for (int i = MagicEffect::Effects::WeaknessToFire; i <= MagicEffect::Effects::WeaknessToNormalWeapons; ++i)
+                effects[i] = MagicEffect::Effects::ResistMagicka;
+
+            effects[MagicEffect::Effects::Burden] = MagicEffect::Effects::ResistMagicka;
+            effects[MagicEffect::Effects::Charm] = MagicEffect::Effects::ResistMagicka;
+            effects[MagicEffect::Effects::Silence] = MagicEffect::Effects::ResistMagicka;
+            effects[MagicEffect::Effects::Blind] = MagicEffect::Effects::ResistMagicka;
+            effects[MagicEffect::Effects::Sound] = MagicEffect::Effects::ResistMagicka;
+
+            for (int i = 0; i < 2; ++i)
+            {
+                effects[MagicEffect::Effects::CalmHumanoid + i] = MagicEffect::Effects::ResistMagicka;
+                effects[MagicEffect::Effects::FrenzyHumanoid + i] = MagicEffect::Effects::ResistMagicka;
+                effects[MagicEffect::Effects::DemoralizeHumanoid + i] = MagicEffect::Effects::ResistMagicka;
+                effects[MagicEffect::Effects::RallyHumanoid + i] = MagicEffect::Effects::ResistMagicka;
+            }
+
+            effects[MagicEffect::Effects::TurnUndead] = MagicEffect::Effects::ResistMagicka;
+
+            effects[MagicEffect::Effects::FireDamage] = MagicEffect::Effects::ResistFire;
+            effects[MagicEffect::Effects::FrostDamage] = MagicEffect::Effects::ResistFrost;
+            effects[MagicEffect::Effects::ShockDamage] = MagicEffect::Effects::ResistShock;
+            effects[MagicEffect::Effects::Vampirism] = MagicEffect::Effects::ResistCommonDisease;
+            effects[MagicEffect::Effects::Corprus] = MagicEffect::Effects::ResistCorprusDisease;
+            effects[MagicEffect::Effects::Poison] = MagicEffect::Effects::ResistPoison;
+            effects[MagicEffect::Effects::Paralyze] = MagicEffect::Effects::ResistParalysis;
+
+            return effects;
+        }
+    }
+
     short MagicEffect::getResistanceEffect(short effect)
     {
         // Source https://wiki.openmw.org/index.php?title=Research:Magic#Effect_attribute
 
         // <Effect, Effect providing resistance against first effect>
-        static std::map<short, short> effects;
+        static const std::map<short, short> effects = makeEffectsMap();
-        if (effects.empty())
-        {
-            effects[DisintegrateArmor] = Sanctuary;
-            effects[DisintegrateWeapon] = Sanctuary;
 
-            for (int i = DrainAttribute; i <= DamageSkill; ++i)
+        if (const auto it = effects.find(effect); it != effects.end())
-                effects[i] = ResistMagicka;
+            return it->second;
-            for (int i = AbsorbAttribute; i <= AbsorbSkill; ++i)
-                effects[i] = ResistMagicka;
-            for (int i = WeaknessToFire; i <= WeaknessToNormalWeapons; ++i)
-                effects[i] = ResistMagicka;
 
-            effects[Burden] = ResistMagicka;
+        return -1;
-            effects[Charm] = ResistMagicka;
-            effects[Silence] = ResistMagicka;
-            effects[Blind] = ResistMagicka;
-            effects[Sound] = ResistMagicka;
-
-            for (int i = 0; i < 2; ++i)
-            {
-                effects[CalmHumanoid + i] = ResistMagicka;
-                effects[FrenzyHumanoid + i] = ResistMagicka;
-                effects[DemoralizeHumanoid + i] = ResistMagicka;
-                effects[RallyHumanoid + i] = ResistMagicka;
-            }
-
-            effects[TurnUndead] = ResistMagicka;
-
-            effects[FireDamage] = ResistFire;
-            effects[FrostDamage] = ResistFrost;
-            effects[ShockDamage] = ResistShock;
-            effects[Vampirism] = ResistCommonDisease;
-            effects[Corprus] = ResistCorprusDisease;
-            effects[Poison] = ResistPoison;
-            effects[Paralyze] = ResistParalysis;
-        }
-
-        if (effects.find(effect) != effects.end())
-            return effects[effect];
-        else
-            return -1;
     }
 
     short MagicEffect::getWeaknessEffect(short effect)

components/sceneutil/attach.cpp

@@ -82,18 +82,32 @@ namespace SceneUtil
         std::string_view mFilter;
     };
 
-    void mergeUserData(const osg::UserDataContainer* source, osg::Object* target)
+    namespace
     {
-        if (!source)
-            return;
 
-        if (!target->getUserDataContainer())
+        void mergeUserData(const osg::UserDataContainer* source, osg::Object* target)
-            target->setUserDataContainer(osg::clone(source, osg::CopyOp::SHALLOW_COPY));
-        else
         {
-            for (unsigned int i = 0; i < source->getNumUserObjects(); ++i)
+            if (!source)
-                target->getUserDataContainer()->addUserObject(
+                return;
-                    osg::clone(source->getUserObject(i), osg::CopyOp::SHALLOW_COPY));
+
+            if (!target->getUserDataContainer())
+                target->setUserDataContainer(osg::clone(source, osg::CopyOp::SHALLOW_COPY));
+            else
+            {
+                for (unsigned int i = 0; i < source->getNumUserObjects(); ++i)
+                    target->getUserDataContainer()->addUserObject(
+                        osg::clone(source->getUserObject(i), osg::CopyOp::SHALLOW_COPY));
+            }
+        }
+
+        osg::ref_ptr<osg::StateSet> makeFrontFaceStateSet()
+        {
+            osg::ref_ptr<osg::FrontFace> frontFace = new osg::FrontFace;
+            frontFace->setMode(osg::FrontFace::CLOCKWISE);
+
+            osg::ref_ptr<osg::StateSet> frontFaceStateSet = new osg::StateSet;
+            frontFaceStateSet->setAttributeAndModes(frontFace, osg::StateAttribute::ON);
+            return frontFaceStateSet;
         }
     }
 
@@ -159,14 +173,8 @@ namespace SceneUtil
                 // Note: for absolute correctness we would need to check the current front face for every mesh then
                 // invert it However MW isn't doing this either, so don't. Assuming all meshes are using backface
                 // culling is more efficient.
-                static osg::ref_ptr<osg::StateSet> frontFaceStateSet;
+                static const osg::ref_ptr<osg::StateSet> frontFaceStateSet = makeFrontFaceStateSet();
-                if (!frontFaceStateSet)
+
-                {
-                    frontFaceStateSet = new osg::StateSet;
-                    osg::FrontFace* frontFace = new osg::FrontFace;
-                    frontFace->setMode(osg::FrontFace::CLOCKWISE);
-                    frontFaceStateSet->setAttributeAndModes(frontFace, osg::StateAttribute::ON);
-                }
                 trans->setStateSet(frontFaceStateSet);
             }
 

components/terrain/quadtreeworld.cpp

@@ -404,63 +404,68 @@ namespace Terrain
         }
     }
 
-    void updateWaterCullingView(
+    namespace
-        HeightCullCallback* callback, ViewData* vd, osgUtil::CullVisitor* cv, float cellworldsize, bool outofworld)
     {
-        if (!(cv->getTraversalMask() & callback->getCullMask()))
+        osg::ref_ptr<osg::StateSet> makeStateSet()
-            return;
-        float lowZ = std::numeric_limits<float>::max();
-        float highZ = callback->getHighZ();
-        if (cv->getEyePoint().z() <= highZ || outofworld)
         {
-            callback->setLowZ(-std::numeric_limits<float>::max());
+            osg::ref_ptr<osg::StateSet> stateSet = new osg::StateSet;
-            return;
+            stateSet->setMode(GL_CULL_FACE, osg::StateAttribute::OFF);
+            stateSet->setMode(GL_DEPTH_TEST, osg::StateAttribute::OFF);
+            stateSet->setAttributeAndModes(
+                new osg::PolygonMode(osg::PolygonMode::FRONT_AND_BACK, osg::PolygonMode::LINE),
+                osg::StateAttribute::ON);
+            osg::ref_ptr<osg::Material> material = new osg::Material;
+            material->setEmission(osg::Material::FRONT_AND_BACK, osg::Vec4f(0, 0, 1, 1));
+            material->setDiffuse(osg::Material::FRONT_AND_BACK, osg::Vec4f(0, 0, 0, 1));
+            material->setAmbient(osg::Material::FRONT_AND_BACK, osg::Vec4f(0, 0, 0, 1));
+            stateSet->setAttributeAndModes(material, osg::StateAttribute::ON);
+            stateSet->setRenderBinDetails(100, "RenderBin");
+            return stateSet;
         }
-        cv->pushCurrentMask();
-        static bool debug = getenv("OPENMW_WATER_CULLING_DEBUG") != nullptr;
-        for (unsigned int i = 0; i < vd->getNumEntries(); ++i)
-        {
-            ViewDataEntry& entry = vd->getEntry(i);
-            osg::BoundingBox bb
-                = static_cast<TerrainDrawable*>(entry.mRenderingNode->asGroup()->getChild(0))->getWaterBoundingBox();
-            if (!bb.valid())
-                continue;
-            osg::Vec3f ofs(
-                entry.mNode->getCenter().x() * cellworldsize, entry.mNode->getCenter().y() * cellworldsize, 0.f);
-            bb._min += ofs;
-            bb._max += ofs;
-            bb._min.z() = highZ;
-            bb._max.z() = highZ;
-            if (cv->isCulled(bb))
-                continue;
-            lowZ = bb._min.z();
 
-            if (!debug)
+        void updateWaterCullingView(
-                break;
+            HeightCullCallback* callback, ViewData* vd, osgUtil::CullVisitor* cv, float cellworldsize, bool outofworld)
-            osg::Box* b = new osg::Box;
+        {
-            b->set(bb.center(), bb._max - bb.center());
+            if (!(cv->getTraversalMask() & callback->getCullMask()))
-            osg::ShapeDrawable* drw = new osg::ShapeDrawable(b);
+                return;
-            static osg::ref_ptr<osg::StateSet> stateset = nullptr;
+            float lowZ = std::numeric_limits<float>::max();
-            if (!stateset)
+            float highZ = callback->getHighZ();
+            if (cv->getEyePoint().z() <= highZ || outofworld)
             {
-                stateset = new osg::StateSet;
+                callback->setLowZ(-std::numeric_limits<float>::max());
-                stateset->setMode(GL_CULL_FACE, osg::StateAttribute::OFF);
+                return;
-                stateset->setMode(GL_DEPTH_TEST, osg::StateAttribute::OFF);
-                stateset->setAttributeAndModes(
-                    new osg::PolygonMode(osg::PolygonMode::FRONT_AND_BACK, osg::PolygonMode::LINE),
-                    osg::StateAttribute::ON);
-                osg::Material* m = new osg::Material;
-                m->setEmission(osg::Material::FRONT_AND_BACK, osg::Vec4f(0, 0, 1, 1));
-                m->setDiffuse(osg::Material::FRONT_AND_BACK, osg::Vec4f(0, 0, 0, 1));
-                m->setAmbient(osg::Material::FRONT_AND_BACK, osg::Vec4f(0, 0, 0, 1));
-                stateset->setAttributeAndModes(m, osg::StateAttribute::ON);
-                stateset->setRenderBinDetails(100, "RenderBin");
             }
-            drw->setStateSet(stateset);
+            cv->pushCurrentMask();
-            drw->accept(*cv);
+            static bool debug = getenv("OPENMW_WATER_CULLING_DEBUG") != nullptr;
+            for (unsigned int i = 0; i < vd->getNumEntries(); ++i)
+            {
+                ViewDataEntry& entry = vd->getEntry(i);
+                osg::BoundingBox bb = static_cast<TerrainDrawable*>(entry.mRenderingNode->asGroup()->getChild(0))
+                                          ->getWaterBoundingBox();
+                if (!bb.valid())
+                    continue;
+                osg::Vec3f ofs(
+                    entry.mNode->getCenter().x() * cellworldsize, entry.mNode->getCenter().y() * cellworldsize, 0.f);
+                bb._min += ofs;
+                bb._max += ofs;
+                bb._min.z() = highZ;
+                bb._max.z() = highZ;
+                if (cv->isCulled(bb))
+                    continue;
+                lowZ = bb._min.z();
+
+                if (!debug)
+                    break;
+                osg::Box* b = new osg::Box;
+                b->set(bb.center(), bb._max - bb.center());
+                osg::ShapeDrawable* drw = new osg::ShapeDrawable(b);
+                static const osg::ref_ptr<osg::StateSet> stateset = makeStateSet();
+                drw->setStateSet(stateset);
+                drw->accept(*cv);
+            }
+            callback->setLowZ(lowZ);
+            cv->popCurrentMask();
         }
-        callback->setLowZ(lowZ);
-        cv->popCurrentMask();
     }
 
     void QuadTreeWorld::accept(osg::NodeVisitor& nv)