FGFS-4.1/OpenSceneGraph
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Gunnar Holm, "After upgrading from 2.8.3 to 3.0.1 we experienced a lock in the Mutex

Browse Source 
functionality when using Terrain::setVerticalScale. This was caused by
the following call sequence resulting in a lockup:

void Terrain::setVerticalScale(float scale)
  CALLS    dirtyRegisteredTiles();


void Terrain::dirtyRegisteredTiles(int dirtyMask)
 SETS LOCK    OpenThreads::ScopedLock<OpenThreads::Mutex> lock(_mutex);
 and CALLS (on every tile)   setDirtyMask(dirtyMask);


void TerrainTile::setDirtyMask(int dirtyMask)
 CALLS _terrain->updateTerrainTileOnNextFrame(this);


void Terrain::updateTerrainTileOnNextFrame(TerrainTile* terrainTile)
 SETS LOCK   OpenThreads::ScopedLock<OpenThreads::Mutex> lock(_mutex);
******* PROBLEM - since lock has already been set! ********


The suggested fix submitted changes from using Mutex to ReentrantMutex.
"
This commit is contained in:
Robert Osfield 2011-12-23 17:34:07 +00:00
parent c7698c1334
commit a53308f7e8
2 changed files with 10 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
include/osgTerrain/Terrain
View File

@ -15,6 +15,7 @@
#define OSGTerrain 1 #define OSGTerrain 1
#include <osg/CoordinateSystemNode> #include <osg/CoordinateSystemNode>
#include <OpenThreads/ReentrantMutex>
#include <osgTerrain/TerrainTile> #include <osgTerrain/TerrainTile>
@ -109,7 +110,7 @@ class OSGTERRAIN_EXPORT Terrain : public osg::CoordinateSystemNode
TerrainTile::BlendingPolicy _blendingPolicy; TerrainTile::BlendingPolicy _blendingPolicy;
bool _equalizeBoundaries; bool _equalizeBoundaries;
mutable OpenThreads::Mutex _mutex; mutable OpenThreads::ReentrantMutex _mutex;
TerrainTileSet _terrainTileSet; TerrainTileSet _terrainTileSet;
TerrainTileMap _terrainTileMap; TerrainTileMap _terrainTileMap;
TerrainTileSet _updateTerrainTileSet; TerrainTileSet _updateTerrainTileSet;

17
src/osgTerrain/Terrain.cpp
View File

@ -44,7 +44,7 @@ Terrain::Terrain(const Terrain& ts, const osg::CopyOp& copyop):
Terrain::~Terrain() Terrain::~Terrain()
{ {
OpenThreads::ScopedLock<OpenThreads::Mutex> lock(_mutex); OpenThreads::ScopedLock<OpenThreads::ReentrantMutex> lock(_mutex);
for(TerrainTileSet::iterator itr = _terrainTileSet.begin(); for(TerrainTileSet::iterator itr = _terrainTileSet.begin();
itr != _terrainTileSet.end(); itr != _terrainTileSet.end();
@ -96,7 +96,7 @@ void Terrain::traverse(osg::NodeVisitor& nv)
typedef std::list< osg::ref_ptr<TerrainTile> > TerrainTileList; typedef std::list< osg::ref_ptr<TerrainTile> > TerrainTileList;
TerrainTileList tiles; TerrainTileList tiles;
{ {
OpenThreads::ScopedLock<OpenThreads::Mutex> lock(_mutex); OpenThreads::ScopedLock<OpenThreads::ReentrantMutex> lock(_mutex);
std::copy(_updateTerrainTileSet.begin(), _updateTerrainTileSet.end(), std::back_inserter(tiles)); std::copy(_updateTerrainTileSet.begin(), _updateTerrainTileSet.end(), std::back_inserter(tiles));
_updateTerrainTileSet.clear(); _updateTerrainTileSet.clear();
} }
@ -116,14 +116,14 @@ void Terrain::traverse(osg::NodeVisitor& nv)
void Terrain::updateTerrainTileOnNextFrame(TerrainTile* terrainTile) void Terrain::updateTerrainTileOnNextFrame(TerrainTile* terrainTile)
{ {
OpenThreads::ScopedLock<OpenThreads::Mutex> lock(_mutex); OpenThreads::ScopedLock<OpenThreads::ReentrantMutex> lock(_mutex);
_updateTerrainTileSet.insert(terrainTile); _updateTerrainTileSet.insert(terrainTile);
} }
TerrainTile* Terrain::getTile(const TileID& tileID) TerrainTile* Terrain::getTile(const TileID& tileID)
{ {
OpenThreads::ScopedLock<OpenThreads::Mutex> lock(_mutex); OpenThreads::ScopedLock<OpenThreads::ReentrantMutex> lock(_mutex);
// OSG_NOTICE<<"Terrain::getTile("<<tileID.level<<", "<<tileID.x<<", "<<tileID.y<<")"<<std::endl; // OSG_NOTICE<<"Terrain::getTile("<<tileID.level<<", "<<tileID.x<<", "<<tileID.y<<")"<<std::endl;
@ -135,7 +135,7 @@ TerrainTile* Terrain::getTile(const TileID& tileID)
const TerrainTile* Terrain::getTile(const TileID& tileID) const const TerrainTile* Terrain::getTile(const TileID& tileID) const
{ {
OpenThreads::ScopedLock<OpenThreads::Mutex> lock(_mutex); OpenThreads::ScopedLock<OpenThreads::ReentrantMutex> lock(_mutex);
TerrainTileMap::const_iterator itr = _terrainTileMap.find(tileID); TerrainTileMap::const_iterator itr = _terrainTileMap.find(tileID);
if (itr == _terrainTileMap.end()) return 0; if (itr == _terrainTileMap.end()) return 0;
@ -145,8 +145,7 @@ const TerrainTile* Terrain::getTile(const TileID& tileID) const
void Terrain::dirtyRegisteredTiles(int dirtyMask) void Terrain::dirtyRegisteredTiles(int dirtyMask)
{ {
OpenThreads::ScopedLock<OpenThreads::Mutex> lock(_mutex); OpenThreads::ScopedLock<OpenThreads::ReentrantMutex> lock(_mutex);
for(TerrainTileSet::iterator itr = _terrainTileSet.begin(); for(TerrainTileSet::iterator itr = _terrainTileSet.begin();
itr != _terrainTileSet.end(); itr != _terrainTileSet.end();
++itr) ++itr)
@ -160,7 +159,7 @@ void Terrain::registerTerrainTile(TerrainTile* tile)
{ {
if (!tile) return; if (!tile) return;
OpenThreads::ScopedLock<OpenThreads::Mutex> lock(_mutex); OpenThreads::ScopedLock<OpenThreads::ReentrantMutex> lock(_mutex);
if (tile->getTileID().valid()) if (tile->getTileID().valid())
{ {
@ -179,7 +178,7 @@ void Terrain::unregisterTerrainTile(TerrainTile* tile)
{ {
if (!tile) return; if (!tile) return;
OpenThreads::ScopedLock<OpenThreads::Mutex> lock(_mutex); OpenThreads::ScopedLock<OpenThreads::ReentrantMutex> lock(_mutex);
if (tile->getTileID().valid()) if (tile->getTileID().valid())
{ {