From Tim Moore,

Fixes to race in DatabasePager where a parent PagedLOD
of newly loaded subgraph has been expired.

Clean up of visitor naming to make it clearer what role it has.

include/osgDB/DatabasePager

@@ -271,7 +271,7 @@ class OSGDB_EXPORT DatabasePager : public osg::NodeVisitor::DatabaseRequestHandl
         typedef std::set< osg::ref_ptr<osg::StateSet> >                 StateSetList;
         typedef std::vector< osg::ref_ptr<osg::Drawable> >              DrawableList;
 
-        class CountPagedLODsVisitor;
+        class ExpirePagedLODsVisitor;
 
         typedef std::list<  osg::ref_ptr<osg::Object> > ObjectList;
 
@@ -307,7 +307,8 @@ class OSGDB_EXPORT DatabasePager : public osg::NodeVisitor::DatabaseRequestHandl
                 _frameNumberLastRequest(0),
                 _timestampLastRequest(0.0),
                 _priorityLastRequest(0.0f),
-                _numOfRequests(0)
+                _numOfRequests(0),
+                _groupExpired(false)
             {}
 
             void invalidate();
@@ -331,6 +332,7 @@ class OSGDB_EXPORT DatabasePager : public osg::NodeVisitor::DatabaseRequestHandl
             osg::ref_ptr<Options>               _loadOptions;
 
             osg::observer_ptr<osgUtil::IncrementalCompileOperation::CompileSet> _compileSet;
+            bool                        _groupExpired; // flag used only in update thread
 
             bool isRequestCurrent (int frameNumber) const
             {

src/osg/PagedLOD.cpp

@@ -288,13 +288,14 @@ bool PagedLOD::removeExpiredChildren(double expiryTime, unsigned int expiryFrame
 {
     if (_children.size()>_numChildrenThatCannotBeExpired)
     {
-        if (!_perRangeDataList[_children.size()-1]._filename.empty() &&
-            _perRangeDataList[_children.size()-1]._timeStamp<expiryTime &&
-            _perRangeDataList[_children.size()-1]._frameNumber<expiryFrame)
+        unsigned cindex = _children.size() - 1;
+        if (!_perRangeDataList[cindex]._filename.empty() &&
+            _perRangeDataList[cindex]._timeStamp<expiryTime &&
+            _perRangeDataList[cindex]._frameNumber<expiryFrame)
         {            
-            osg::Node* nodeToRemove = _children[_children.size()-1].get();
+            osg::Node* nodeToRemove = _children[cindex].get();
             removedChildren.push_back(nodeToRemove);
-            return Group::removeChildren(_children.size()-1,1);
+            return Group::removeChildren(cindex,1);
         }
     }
     return false;

src/osgDB/DatabasePager.cpp

@@ -129,29 +129,30 @@ void DatabasePager::compileCompleted(DatabaseRequest* databaseRequest)
     _dataToMergeList->add(databaseRequest);
 }
 
-/////////////////////////////////////////////////////////////////////////////////////////////////////////////////
-//
-//  CountPagedLODList
-//
-class DatabasePager::CountPagedLODsVisitor : public osg::NodeVisitor
+// This class is a helper for the management of SetBasedPagedLODList.
+class DatabasePager::ExpirePagedLODsVisitor : public osg::NodeVisitor
 {
 public:
-    CountPagedLODsVisitor():
-        osg::NodeVisitor(osg::NodeVisitor::TRAVERSE_ALL_CHILDREN),
-        _numPagedLODs(0)
+    ExpirePagedLODsVisitor():
+        osg::NodeVisitor(osg::NodeVisitor::TRAVERSE_ALL_CHILDREN)
     {
     }
 
-    META_NodeVisitor("osgDB","CountPagedLODsVisitor")
+    META_NodeVisitor("osgDB","ExpirePagedLODsVisitor")
 
     virtual void apply(osg::PagedLOD& plod)
     {
-        ++_numPagedLODs;
-        _pagedLODs.insert(&plod);
+        _childPagedLODs.insert(&plod);
+        markRequestsExpired(&plod);
         traverse(plod);
     }
 
-    bool removeExpiredChildrenAndCountPagedLODs(osg::PagedLOD* plod, double expiryTime, unsigned int expiryFrame, osg::NodeList& removedChildren)
+    // Remove expired children from a PagedLOD. On return
+    // removedChildren contains the nodes removed by the call to
+    // PagedLOD::removeExpiredChildren, and the _childPagedLODs member
+    // contains all the PagedLOD objects found in those children's
+    // subgraphs.
+    bool removeExpiredChildrenAndFindPagedLODs(osg::PagedLOD* plod, double expiryTime, unsigned int expiryFrame, osg::NodeList& removedChildren)
     {
         size_t sizeBefore = removedChildren.size();
 
@@ -161,24 +162,25 @@ class DatabasePager::CountPagedLODsVisitor : public osg::NodeVisitor
         {
             removedChildren[i]->accept(*this);
         }
-
-        for(PagedLODset::iterator itr = _pagedLODs.begin();
-            itr != _pagedLODs.end();
-            ++itr)
-        {
-            removedChildren.push_back(*itr);
-        }
-
         return sizeBefore!=removedChildren.size();
     }
 
-
     typedef std::set<osg::ref_ptr<osg::PagedLOD> > PagedLODset;
-    PagedLODset         _pagedLODs;
-    int                 _numPagedLODs;
+    PagedLODset         _childPagedLODs;
+private:
+    void markRequestsExpired(osg::PagedLOD* plod)
+    {
+        unsigned numFiles = plod->getNumFileNames();
+        for (unsigned i = 0; i < numFiles; ++i)
+        {
+            DatabasePager::DatabaseRequest* request
+                = dynamic_cast<DatabasePager::DatabaseRequest*>(plod->getDatabaseRequest(i).get());
+            if (request)
+                request->_groupExpired = true;
+        }
+    }
 };
 
-
 /////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 //
 //  SetBasedPagedLODList
@@ -201,7 +203,7 @@ class SetBasedPagedLODList : public DatabasePager::PagedLODList
     {
         int leftToRemove = numberChildrenToRemove;
         for(PagedLODs::iterator itr = _pagedLODs.begin();
-            itr!=_pagedLODs.end() && leftToRemove >= 0;
+            itr!=_pagedLODs.end() && leftToRemove > 0;
             )
         {
             osg::ref_ptr<osg::PagedLOD> plod;
@@ -210,14 +212,14 @@ class SetBasedPagedLODList : public DatabasePager::PagedLODList
                 bool plodActive = expiryFrame < plod->getFrameNumberOfLastTraversal();
                 if (visitActive==plodActive) // true if (visitActive && plodActive) OR (!visitActive &&!plodActive)
                 {
-                    DatabasePager::CountPagedLODsVisitor countPagedLODsVisitor;
+                    DatabasePager::ExpirePagedLODsVisitor expirePagedLODsVisitor;
                     osg::NodeList expiredChildren; // expired PagedLODs
-                    countPagedLODsVisitor.removeExpiredChildrenAndCountPagedLODs(
+                    expirePagedLODsVisitor.removeExpiredChildrenAndFindPagedLODs(
                         plod.get(), expiryTime, expiryFrame, expiredChildren);
                     // Clear any expired PagedLODs out of the set
-                    for (DatabasePager::CountPagedLODsVisitor::PagedLODset::iterator
-                             citr = countPagedLODsVisitor._pagedLODs.begin(),
-                             end = countPagedLODsVisitor._pagedLODs.end();
+                    for (DatabasePager::ExpirePagedLODsVisitor::PagedLODset::iterator
+                             citr = expirePagedLODsVisitor._childPagedLODs.begin(),
+                             end = expirePagedLODsVisitor._childPagedLODs.end();
                          citr != end;
                         ++citr)
                     {
@@ -1588,7 +1590,7 @@ void DatabasePager::addLoadedDataToSceneGraph(const osg::FrameStamp &frameStamp)
         // the request; the cull traversal -- which might redispatch
         // the request -- can't run at the sametime as this update traversal.
         osg::ref_ptr<osg::Group> group;
-        if (databaseRequest->_group.lock(group))
+        if (!databaseRequest->_groupExpired && databaseRequest->_group.lock(group))
         {
             if (osgDB::Registry::instance()->getSharedStateManager())
                 osgDB::Registry::instance()->getSharedStateManager()->share(databaseRequest->_loadedModel.get());