From 2e154d5976b6c9d62880283485ab44cac2460003 Mon Sep 17 00:00:00 2001 From: Robert Osfield Date: Fri, 26 Feb 2010 09:23:28 +0000 Subject: [PATCH] From Ryan Kawicki, "I guess I missed these during my testing, but if the database pager has outstanding requests while the application is shutting down, the archive can become invalidated through unsafe calls to ReaderWriterTXP::getArchive. I've made this function return a ref_ptr and change other locations to as needed to conform to the change. I've tested this and no more crashes. Following files from revision 11057 have been attached." --- src/osgPlugins/txp/ReaderWriterTXP.cpp | 20 +++---- src/osgPlugins/txp/ReaderWriterTXP.h | 2 +- src/osgPlugins/txp/TXPNode.cpp | 5 -- src/osgPlugins/txp/TXPNode.h | 2 +- src/osgPlugins/txp/TXPParser.h | 72 +++++++++++++------------- 5 files changed, 48 insertions(+), 53 deletions(-) diff --git a/src/osgPlugins/txp/ReaderWriterTXP.cpp b/src/osgPlugins/txp/ReaderWriterTXP.cpp index 041c59480..b21135baa 100644 --- a/src/osgPlugins/txp/ReaderWriterTXP.cpp +++ b/src/osgPlugins/txp/ReaderWriterTXP.cpp @@ -58,7 +58,7 @@ osgDB::ReaderWriter::ReadResult ReaderWriterTXP::local_readNode(const std::strin //we will set our osgdb loader options on the archive and set the appropriate archive on //the txpNode. int id = ++_archiveId; - TXPArchive* archive = getArchive(id,osgDB::getFilePath(fileName)); + osg::ref_ptr< TXPArchive > archive = getArchive(id,osgDB::getFilePath(fileName)); if (archive != NULL) { @@ -69,8 +69,8 @@ osgDB::ReaderWriter::ReadResult ReaderWriterTXP::local_readNode(const std::strin archive->SetMaterialAttributesToStateSetVar(true); } - txpNode->loadArchive(archive); - + txpNode->loadArchive(archive.get()); + return txpNode.get(); } else @@ -85,7 +85,7 @@ osgDB::ReaderWriter::ReadResult ReaderWriterTXP::local_readNode(const std::strin int x,y,lod; unsigned int id; sscanf(name.c_str(),"tile%d_%dx%d_%u",&lod,&x,&y,&id); - TXPArchive* archive = getArchive(id,osgDB::getFilePath(file)); + osg::ref_ptr< TXPArchive > archive = getArchive(id,osgDB::getFilePath(file)); // The way this is done a 'tile' should only be created for lod 0 only, // something is wrong if this is no the case @@ -106,7 +106,7 @@ osgDB::ReaderWriter::ReadResult ReaderWriterTXP::local_readNode(const std::strin return ReadResult::ERROR_IN_READING_FILE; std::vector childrenLoc; - osg::ref_ptr tileContent = getTileContent(info,x,y,lod,archive, childrenLoc); + osg::ref_ptr tileContent = getTileContent(info,x,y,lod,archive.get(), childrenLoc); tileContent->setName("TileContent"); @@ -198,7 +198,7 @@ osgDB::ReaderWriter::ReadResult ReaderWriterTXP::local_readNode(const std::strin int x,y,lod; unsigned int id; sscanf(name.c_str(),"subtiles%d_%dx%d_%u",&lod,&x,&y,&id); - TXPArchive* archive = getArchive(id,osgDB::getFilePath(file)); + osg::ref_ptr< TXPArchive > archive = getArchive(id,osgDB::getFilePath(file)); int majorVersion, minorVersion; archive->GetVersion(majorVersion, minorVersion); @@ -258,7 +258,7 @@ osgDB::ReaderWriter::ReadResult ReaderWriterTXP::local_readNode(const std::strin if (!archive->getTileInfo(loc,info)) continue; - osg::ref_ptr tileContent = getTileContent(info, loc, archive, childrenChildLoc); + osg::ref_ptr tileContent = getTileContent(info, loc, archive.get(), childrenChildLoc); tileContent->setName("TileContent"); @@ -360,7 +360,7 @@ osgDB::ReaderWriter::ReadResult ReaderWriterTXP::local_readNode(const std::strin if (!archive->getTileInfo(tileX,tileY,tileLOD,info)) continue; - osg::ref_ptr tileContent = getTileContent(info,tileX,tileY,tileLOD,archive, childrenLoc); + osg::ref_ptr tileContent = getTileContent(info,tileX,tileY,tileLOD,archive.get(), childrenLoc); tileContent->setName("TileContent"); @@ -563,9 +563,9 @@ bool ReaderWriterTXP::extractChildrenLocations(const std::string& name, int pare } -TXPArchive *ReaderWriterTXP::getArchive(int id, const std::string& dir) +osg::ref_ptr< TXPArchive > ReaderWriterTXP::getArchive(int id, const std::string& dir) { - TXPArchive* archive = NULL; + osg::ref_ptr< TXPArchive > archive = NULL; std::map< int,osg::ref_ptr >::iterator iter = _archives.find(id); diff --git a/src/osgPlugins/txp/ReaderWriterTXP.h b/src/osgPlugins/txp/ReaderWriterTXP.h index e50409d74..41646b7fa 100644 --- a/src/osgPlugins/txp/ReaderWriterTXP.h +++ b/src/osgPlugins/txp/ReaderWriterTXP.h @@ -81,7 +81,7 @@ protected: ReadResult local_readNode(const std::string& file, const osgDB::ReaderWriter::Options* options); - TXPArchive *getArchive(int id, const std::string&); + osg::ref_ptr< TXPArchive > getArchive(int id, const std::string&); osg::Node* getTileContent(const TXPArchive::TileInfo &info, int x, int y, int lod, TXPArchive* archive, std::vector& childrenLoc); osg::Node* getTileContent(const TXPArchive::TileInfo &info, const TXPArchive::TileLocationInfo& loc, TXPArchive* archive, std::vector& childrenLoc); diff --git a/src/osgPlugins/txp/TXPNode.cpp b/src/osgPlugins/txp/TXPNode.cpp index 313b433a9..e20614b2a 100644 --- a/src/osgPlugins/txp/TXPNode.cpp +++ b/src/osgPlugins/txp/TXPNode.cpp @@ -98,11 +98,6 @@ TXPNode::~TXPNode() } } -TXPArchive* TXPNode::getArchive() -{ - return _archive.get(); -} - void TXPNode::traverse(osg::NodeVisitor& nv) { switch(nv.getVisitorType()) diff --git a/src/osgPlugins/txp/TXPNode.h b/src/osgPlugins/txp/TXPNode.h index ad82f6bcc..891a250f2 100644 --- a/src/osgPlugins/txp/TXPNode.h +++ b/src/osgPlugins/txp/TXPNode.h @@ -71,7 +71,7 @@ public: //If NULL is passed into loadArchive it will do the same thing it used to. bool loadArchive(TXPArchive*); - TXPArchive* getArchive(); + TXPArchive* getArchive() { return _archive.get(); } void setArchive( TXPArchive* archive ) { diff --git a/src/osgPlugins/txp/TXPParser.h b/src/osgPlugins/txp/TXPParser.h index 9aaf5a03d..43e328572 100644 --- a/src/osgPlugins/txp/TXPParser.h +++ b/src/osgPlugins/txp/TXPParser.h @@ -79,11 +79,11 @@ class GeodeGroup : public osg::Group public: GeodeGroup() : osg::Group(), _geode(NULL) - {} + {} GeodeGroup(const GeodeGroup& gg,const osg::CopyOp& copyop=osg::CopyOp::SHALLOW_COPY): osg::Group(gg, copyop), _geode(gg._geode) - {} + {} META_Node(txp, GeodeGroup); @@ -117,19 +117,19 @@ public: _archive = archive; } - // Gets the archive - inline TXPArchive* getArchive() - { - return _archive; - } - + // Gets the archive + inline TXPArchive* getArchive() + { + return _archive.get(); + } + // Scene parser osg::Group *parseScene( trpgReadBuffer &buf, std::map > &materials, std::map > &models, double realMinRange, double realMaxRange, double usedMaxRange); - + // Returns the current Top Group inline osg::Group* getCurrTop() { @@ -148,11 +148,11 @@ public: return _materialMap; } - // Ensure material is loaded - inline void loadMaterial( int ix ) - { - _archive->loadMaterial( ix ); - } + // Ensure material is loaded + inline void loadMaterial( int ix ) + { + _archive->loadMaterial( ix ); + } // New to TerraPage 2.0 - local materials std::vector >* getLocalMaterials() @@ -269,15 +269,15 @@ public: } // gets tile center, from the top lod node - inline const osg::Vec3 getTileCenter() const - { - return _tileCenter; - } + inline const osg::Vec3 getTileCenter() const + { + return _tileCenter; + } - inline void setCurrentNode( osg::Node* node ) - { - _currentNode = node; - } + inline void setCurrentNode( osg::Node* node ) + { + _currentNode = node; + } // After parsing this will return the number of trpgChildRef node found. unsigned int GetNbChildrenRef() const; @@ -302,7 +302,7 @@ protected: bool EndChildren(void *); // THE archive - TXPArchive *_archive; + osg::ref_ptr< TXPArchive > _archive; // Current parent osg::Group* _currentTop; @@ -323,14 +323,14 @@ protected: void replaceTileLod(osg::Group*); // Materials - typedef std::map >* MaterialMapType; - MaterialMapType _materialMap; + typedef std::map >* MaterialMapType; + MaterialMapType _materialMap; // Local materials std::vector > _localMaterials; // Model list - typedef std::map > OSGModelsMapType; + typedef std::map > OSGModelsMapType; OSGModelsMapType* _models; // Tile header @@ -383,7 +383,7 @@ class geomRead : public trpgr_Callback public: geomRead(TXPParser *in_parse) : _parse(in_parse) - {} + {} void *Parse(trpgToken tok,trpgReadBuffer &buf); protected: TXPParser *_parse; @@ -395,7 +395,7 @@ class groupRead : public trpgr_Callback { public: groupRead(TXPParser *in_parse) : _parse(in_parse) - {} + {} void *Parse(trpgToken tok,trpgReadBuffer &buf); protected: TXPParser *_parse; @@ -406,7 +406,7 @@ class attachRead : public trpgr_Callback { public: attachRead(TXPParser *in_parse) : _parse(in_parse) - {} + {} void *Parse(trpgToken tok,trpgReadBuffer &buf); protected: TXPParser *_parse; @@ -455,7 +455,7 @@ class lodRead : public trpgr_Callback { public: lodRead(TXPParser *in_parse) : _parse(in_parse) - {} + {} void *Parse(trpgToken tok,trpgReadBuffer &buf); protected: TXPParser *_parse; @@ -466,7 +466,7 @@ class tileHeaderRead : public trpgr_Callback { public: tileHeaderRead(TXPParser *in_parse) : _parse(in_parse) - {} + {} void *Parse(trpgToken tok,trpgReadBuffer &buf); protected: TXPParser *_parse; @@ -478,7 +478,7 @@ class modelRefRead : public trpgr_Callback { public: modelRefRead(TXPParser *in_parse) : _parse(in_parse) - {} + {} void *Parse(trpgToken tok,trpgReadBuffer &buf); protected: TXPParser *_parse; @@ -489,7 +489,7 @@ class billboardRead : public trpgr_Callback { public: billboardRead(TXPParser *in_parse) : _parse(in_parse) - {} + {} void *Parse(trpgToken tok,trpgReadBuffer &buf); protected: TXPParser *_parse; @@ -500,7 +500,7 @@ class lightRead: public trpgr_Callback { public: lightRead(TXPParser *in_parse) : _parse(in_parse) - {} + {} void *Parse(trpgToken tok,trpgReadBuffer &buf); protected: TXPParser *_parse; @@ -511,7 +511,7 @@ class layerRead: public trpgr_Callback { public: layerRead(TXPParser *in_parse) : _parse(in_parse) - {} + {} void *Parse(trpgToken tok,trpgReadBuffer &buf); protected: TXPParser *_parse; @@ -522,7 +522,7 @@ class labelRead: public trpgr_Callback { public: labelRead(TXPParser *in_parse) : _parse(in_parse) - {} + {} void *Parse(trpgToken tok,trpgReadBuffer &buf); protected: TXPParser *_parse;