From 9ad070871afb134a76a3549c8716b0a4f929ad3a Mon Sep 17 00:00:00 2001 From: ThorstenB Date: Mon, 2 Apr 2012 20:09:02 +0200 Subject: [PATCH] Use observer_ptr::lock for thread-safe pointer retrieval. Also revert to using ref_ptr for the top-level EffectMap, since it holds elements no one else references (and don't affect memory much). --- simgear/scene/material/TextureBuilder.cxx | 12 ++++++------ simgear/scene/material/makeEffect.cxx | 14 +++++++------- simgear/scene/material/mat.cxx | 5 ++++- simgear/scene/sky/newcloud.cxx | 7 ++----- simgear/scene/tgdb/TreeBin.cxx | 6 ++---- simgear/scene/tgdb/pt_lights.cxx | 9 ++++++--- 6 files changed, 27 insertions(+), 26 deletions(-) diff --git a/simgear/scene/material/TextureBuilder.cxx b/simgear/scene/material/TextureBuilder.cxx index 679d67e3..c06883aa 100644 --- a/simgear/scene/material/TextureBuilder.cxx +++ b/simgear/scene/material/TextureBuilder.cxx @@ -291,20 +291,20 @@ Texture* TexBuilder::build(Effect* effect, Pass* pass, const SGPropertyNode* TexTuple attrs = makeTexTuple(effect, props, options, _type); typename TexMap::iterator itr = texMap.find(attrs); - if (itr != texMap.end()) + ref_ptr tex; + if ((itr != texMap.end())&& + (itr->second.lock(tex))) { - T* tex = itr->second.get(); - if (tex) - return tex; + return tex.release(); } - T* tex = new T; + tex = new T; setAttrs(attrs, tex, options); if (itr == texMap.end()) texMap.insert(make_pair(attrs, tex)); else itr->second = tex; // update existing, but empty observer - return tex; + return tex.release(); } diff --git a/simgear/scene/material/makeEffect.cxx b/simgear/scene/material/makeEffect.cxx index 4ca288b8..06bf47b2 100644 --- a/simgear/scene/material/makeEffect.cxx +++ b/simgear/scene/material/makeEffect.cxx @@ -44,7 +44,7 @@ using namespace osg; using namespace effect; typedef vector RawPropVector; -typedef map > EffectMap; +typedef map > EffectMap; namespace { @@ -206,10 +206,10 @@ Effect* makeEffect(SGPropertyNode* prop, lock(effectMutex); cache = parent->getCache(); itr = cache->find(key); - if (itr != cache->end()) { - effect = itr->second.get(); - if (effect.valid()) - effect->generator = parent->generator; // Copy the generators + if ((itr != cache->end())&& + itr->second.lock(effect)) + { + effect->generator = parent->generator; // Copy the generators } } if (!effect.valid()) { @@ -222,8 +222,8 @@ Effect* makeEffect(SGPropertyNode* prop, pair irslt = cache->insert(make_pair(key, effect)); if (!irslt.second) { - ref_ptr old = irslt.first->second.get(); - if (old.valid()) + ref_ptr old; + if (irslt.first->second.lock(old)) effect = old; // Another thread beat us in creating it! Discard our own... else irslt.first->second = effect; // update existing, but empty observer diff --git a/simgear/scene/material/mat.cxx b/simgear/scene/material/mat.cxx index 582571f3..ec5164bd 100644 --- a/simgear/scene/material/mat.cxx +++ b/simgear/scene/material/mat.cxx @@ -378,6 +378,8 @@ SGMaterial::init () Effect* SGMaterial::get_effect(int i) { if(!_status[i].effect_realized) { + if (!_status[i].effect.valid()) + return 0; _status[i].effect->realizeTechniques(_status[i].options.get()); _status[i].effect_realized = true; } @@ -459,7 +461,8 @@ void SGMaterial::buildEffectProperties(const SGReaderWriterOptions* options) makeChild(effectParamProp, "light-coverage")->setDoubleValue(light_coverage); matState.effect = makeEffect(effectProp, false, options); - matState.effect->setUserData(user.get()); + if (matState.effect.valid()) + matState.effect->setUserData(user.get()); } } diff --git a/simgear/scene/sky/newcloud.cxx b/simgear/scene/sky/newcloud.cxx index 02f04c19..2f24298c 100644 --- a/simgear/scene/sky/newcloud.cxx +++ b/simgear/scene/sky/newcloud.cxx @@ -103,11 +103,8 @@ SGNewCloud::SGNewCloud(const SGPath &texture_root, const SGPropertyNode *cld_def // Create a new Effect for the texture, if required. EffectMap::iterator iter = effectMap.find(texture); - if (iter != effectMap.end()) { - effect = iter->second.get(); - } - - if (!effect.valid()) + if ((iter == effectMap.end())|| + (!iter->second.lock(effect))) { SGPropertyNode_ptr pcloudEffect = new SGPropertyNode; makeChild(pcloudEffect, "inherits-from")->setValue("Effects/cloud"); diff --git a/simgear/scene/tgdb/TreeBin.cxx b/simgear/scene/tgdb/TreeBin.cxx index a0203ebc..b0ca2e02 100644 --- a/simgear/scene/tgdb/TreeBin.cxx +++ b/simgear/scene/tgdb/TreeBin.cxx @@ -302,10 +302,8 @@ osg::Group* createForest(SGTreeBinList& forestList, const osg::Matrix& transform ref_ptr effect; EffectMap::iterator iter = treeEffectMap.find(forest->texture); - if (iter != treeEffectMap.end()) - effect = iter->second.get(); - - if (!effect.valid()) + if ((iter == treeEffectMap.end())|| + (!iter->second.lock(effect))) { SGPropertyNode_ptr effectProp = new SGPropertyNode; makeChild(effectProp, "inherits-from")->setStringValue("Effects/tree"); diff --git a/simgear/scene/tgdb/pt_lights.cxx b/simgear/scene/tgdb/pt_lights.cxx index e4ec0d59..2aa6606a 100644 --- a/simgear/scene/tgdb/pt_lights.cxx +++ b/simgear/scene/tgdb/pt_lights.cxx @@ -174,9 +174,12 @@ Effect* getLightEffect(float size, const Vec3& attenuation, PointParams pointParams(size, attenuation, minSize, maxSize, directional); ScopedLock lock(lightMutex); EffectMap::iterator eitr = effectMap.find(pointParams); - if ((eitr != effectMap.end())&& - eitr->second.valid()) - return eitr->second.get(); + if (eitr != effectMap.end()) + { + ref_ptr effect; + if (eitr->second.lock(effect)) + return effect.release(); + } // Basic stuff; no sprite or attenuation support Pass *basicPass = new Pass; basicPass->setRenderBinDetails(POINT_LIGHTS_BIN, "DepthSortedBin");