Refactored the osg::Referenced observerset code so that it now uses a dedicated ObserverSet class,

which utilises a global recursive mutex that is dedicated to manage Observer and ObserverSet.

The new global mutex for observers avoids problems with deadlocks that were occurring previously when
an osg::Refenced object was being deleted at the same time as on osg::ObserverNodePath.
This commit is contained in:
Robert Osfield 2010-02-18 21:21:12 +00:00
parent 6196652ef0
commit b09757bdb8
7 changed files with 234 additions and 215 deletions

View File

@ -11,17 +11,26 @@
* OpenSceneGraph Public License for more details.
*/
#ifndef OSG_OCCLUDER
#define OSG_OCCLUDER 1
#ifndef OSG_OBSERVER
#define OSG_OBSERVER 1
#include <OpenThreads/Mutex>
#include <osg/Export>
#include <set>
namespace osg {
/** Observer base class for tracking when objects are unreferenced (there reference count goes to 0) and are being deleted.*/
class Observer
class OSG_EXPORT Observer
{
public:
virtual ~Observer() {}
/** Get the optional global observer mutex, this can be shared between all osg::Observer.*/
static OpenThreads::Mutex* getGlobalObserverMutex();
inline OpenThreads::Mutex* getObserverMutex() const { return osg::Observer::getGlobalObserverMutex(); }
/** objectUnreferenced(void*) is called when the observed object's referenced count goes to zero, indicating that
* the object will be deleted unless a new reference is made to it. If you wish to prevent deletion of the object
* then it's reference count should be incremented such as via taking a ref_ptr<> to it, if no refernce is taken
@ -33,6 +42,32 @@ public:
* removed from the observerd objects observer set so there is no need for the objectDeleted implementation
* to call removeObserver() on the observed object. */
virtual void objectDeleted(void*) {}
};
/** Class used by osg::Referenced to track the observers assoicated with it.*/
class OSG_EXPORT ObserverSet
{
public:
ObserverSet();
~ObserverSet();
inline OpenThreads::Mutex* getObserverSetMutex() const { return osg::Observer::getGlobalObserverMutex(); }
void addObserver(Observer* observer);
void removeObserver(Observer* observer);
void signalObjectUnreferenced(void* ptr);
void signalObjectDeleted(void* ptr);
typedef std::set<Observer*> Observers;
Observers& getObservers() { return _observers; }
const Observers& getObservers() const { return _observers; }
protected:
Observers _observers;
};
}

View File

@ -67,7 +67,6 @@ class OSG_EXPORT ObserverNodePath : public osg::Observer
virtual bool objectUnreferenced(void* ptr);
mutable OpenThreads::Mutex _mutex;
osg::NodePath _nodePath;
bool _valid;
};

View File

@ -14,8 +14,7 @@
#ifndef OSG_REFERENCED
#define OSG_REFERENCED 1
// When building OSG with Java need to derive from Noodle::CBridgable class,
#include <osg/Export>
#include <osg/Observer>
#include <OpenThreads/ScopedLock>
#include <OpenThreads/Mutex>
@ -30,6 +29,7 @@ namespace osg {
// forward declare, declared after Referenced below.
class DeleteHandler;
class Observer;
class ObserverSet;
/** template class to help enforce static initialization order. */
template <typename T, T M()>
@ -95,11 +95,25 @@ class OSG_EXPORT Referenced
/** Return the number pointers currently referencing this object. */
inline int referenceCount() const { return _refCount; }
/** Add a Observer that is observing this object, notify the Observer when this object gets deleted.*/
void addObserver(Observer* observer) const;
/** Get the ObserverSet if one is attached, otherwise return NULL.*/
ObserverSet* getObserverSet() const
{
#if defined(_OSG_REFERENCED_USE_ATOMIC_OPERATIONS)
return static_cast<ObserverSet*>(_observerSet.get());
#else
return static_cast<ObserverSet*>(_observerSet);
#endif
}
/** Get the ObserverSet if one is attached, otherwise create an ObserverSet, attach it, then return this newly created ObserverSet.*/
ObserverSet* getOrCreateObserverSet() const;
/** Add a Observer that is observing this object, notify the Observer when this object gets deleted.*/
void removeObserver(Observer* observer) const;
void addObserver(Observer* observer) const { getOrCreateObserverSet()->addObserver(observer); }
/** remove Observer that is observing this object.*/
void removeObserver(Observer* observer) const { getOrCreateObserverSet()->removeObserver(observer); }
public:
@ -128,9 +142,7 @@ class OSG_EXPORT Referenced
void deleteUsingDeleteHandler() const;
#if defined(_OSG_REFERENCED_USE_ATOMIC_OPERATIONS)
struct ObserverSetData;
mutable OpenThreads::AtomicPtr _observerSetDataPtr;
mutable OpenThreads::AtomicPtr _observerSet;
mutable OpenThreads::Atomic _refCount;
#else
@ -139,7 +151,7 @@ class OSG_EXPORT Referenced
mutable int _refCount;
mutable void* _observers;
mutable void* _observerSet;
#endif
};
@ -163,17 +175,17 @@ inline void Referenced::ref() const
inline void Referenced::unref() const
{
#if defined(_OSG_REFERENCED_USE_ATOMIC_OPERATIONS)
bool needDelete = (--_refCount <= 0);
bool needDelete = ((--_refCount) == 0);
#else
bool needDelete = false;
if (_refMutex)
{
OpenThreads::ScopedLock<OpenThreads::Mutex> lock(*_refMutex);
needDelete = (--_refCount)<=0;
needDelete = (--_refCount)==0;
}
else
{
needDelete = (--_refCount)<=0;
needDelete = (--_refCount)==0;
}
#endif

View File

@ -276,6 +276,7 @@ ADD_LIBRARY(${LIB_NAME}
NodeVisitor.cpp
Notify.cpp
Object.cpp
Observer.cpp
ObserverNodePath.cpp
OccluderNode.cpp
OcclusionQueryNode.cpp

71
src/osg/Observer.cpp Normal file
View File

@ -0,0 +1,71 @@
/* -*-c++-*- OpenSceneGraph - Copyright (C) 1998-2006 Robert Osfield
*
* This library is open source and may be redistributed and/or modified under
* the terms of the OpenSceneGraph Public License (OSGPL) version 0.0 or
* (at your option) any later version. The full license is in LICENSE file
* included with this distribution, and on the openscenegraph.org website.
*
* This library is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* OpenSceneGraph Public License for more details.
*/
#include <osg/ObserverNodePath>
#include <osg/Notify>
using namespace osg;
ObserverSet::ObserverSet()
{
//OSG_NOTICE<<"ObserverSet::ObserverSet() "<<this<<std::endl;
}
ObserverSet::~ObserverSet()
{
//OSG_NOTICE<<"ObserverSet::~ObserverSet() "<<this<<", _observers.size()="<<_observers.size()<<std::endl;
}
void ObserverSet::addObserver(Observer* observer)
{
//OSG_NOTICE<<"ObserverSet::addObserver("<<observer<<") "<<this<<std::endl;
OpenThreads::ScopedLock<OpenThreads::Mutex> lock(*getObserverSetMutex());
_observers.insert(observer);
}
void ObserverSet::removeObserver(Observer* observer)
{
//OSG_NOTICE<<"ObserverSet::removeObserver("<<observer<<") "<<this<<std::endl;
OpenThreads::ScopedLock<OpenThreads::Mutex> lock(*getObserverSetMutex());
_observers.erase(observer);
}
void ObserverSet::signalObjectUnreferenced(void* ptr)
{
for(Observers::iterator itr = _observers.begin();
itr != _observers.end();
)
{
if ((*itr)->objectUnreferenced(ptr))
{
Observers::iterator orig_itr = itr;
++itr;
_observers.erase(orig_itr);
}
else
{
++itr;
}
}
}
void ObserverSet::signalObjectDeleted(void* ptr)
{
for(Observers::iterator itr = _observers.begin();
itr != _observers.end();
++itr)
{
(*itr)->objectDeleted(ptr);
}
_observers.clear();
}

View File

@ -82,7 +82,7 @@ void ObserverNodePath::setNodePathTo(osg::Node* node)
void ObserverNodePath::setNodePath(const osg::NodePath& nodePath)
{
OpenThreads::ScopedLock<OpenThreads::Mutex> lock(_mutex);
OpenThreads::ScopedLock<OpenThreads::Mutex> lock(*getObserverMutex());
_setNodePath(nodePath);
}
@ -98,7 +98,7 @@ void ObserverNodePath::setNodePath(const osg::RefNodePath& refNodePath)
void ObserverNodePath::clearNodePath()
{
OpenThreads::ScopedLock<OpenThreads::Mutex> lock(_mutex);
OpenThreads::ScopedLock<OpenThreads::Mutex> lock(*getObserverMutex());
_clearNodePath();
}
@ -106,7 +106,7 @@ bool ObserverNodePath::getRefNodePath(RefNodePath& refNodePath) const
{
refNodePath.clear();
OpenThreads::ScopedLock<OpenThreads::Mutex> lock(_mutex);
OpenThreads::ScopedLock<OpenThreads::Mutex> lock(*getObserverMutex());
if (!_valid) return false;
for(osg::NodePath::const_iterator itr = _nodePath.begin();
@ -123,7 +123,7 @@ bool ObserverNodePath::getNodePath(NodePath& nodePath) const
{
nodePath.clear();
OpenThreads::ScopedLock<OpenThreads::Mutex> lock(_mutex);
OpenThreads::ScopedLock<OpenThreads::Mutex> lock(*getObserverMutex());
if (!_valid) return false;
nodePath = _nodePath;
return true;
@ -168,7 +168,7 @@ bool ObserverNodePath::objectUnreferenced(void* ptr)
{
osg::Node* node = reinterpret_cast<osg::Node*>(ptr);
OpenThreads::ScopedLock<OpenThreads::Mutex> lock(_mutex);
OpenThreads::ScopedLock<OpenThreads::Mutex> lock(*getObserverMutex());
_valid = false;

View File

@ -15,7 +15,7 @@
#include <osg/Referenced>
#include <osg/Notify>
#include <osg/ApplicationUsage>
#include <osg/observer_ptr>
#include <osg/Observer>
#include <typeinfo>
#include <memory>
@ -87,14 +87,12 @@ OpenThreads::Mutex* Referenced::getGlobalReferencedMutex()
return s_ReferencedGlobalMutext.get();
}
typedef std::set<Observer*> ObserverSet;
#if defined(_OSG_REFERENCED_USE_ATOMIC_OPERATIONS)
struct Referenced::ObserverSetData {
OpenThreads::Mutex _mutex;
ObserverSet _observers;
};
#endif
// we'll implement Observer::getGlobalObserverMutex() here for convinience as ResetPointer is available.
OpenThreads::Mutex* Observer::getGlobalObserverMutex()
{
static GlobalMutexPointer s_ReferencedGlobalMutext = new OpenThreads::Mutex(OpenThreads::Mutex::MUTEX_RECURSIVE);
return s_ReferencedGlobalMutext.get();
}
#if !defined(_OSG_REFERENCED_USE_ATOMIC_OPERATIONS)
static bool s_useThreadSafeReferenceCounting = getenv("OSG_THREAD_SAFE_REF_UNREF")!=0;
@ -142,12 +140,12 @@ static int s_numObjects = 0;
Referenced::Referenced():
#if defined(_OSG_REFERENCED_USE_ATOMIC_OPERATIONS)
_observerSetDataPtr(0),
_observerSet(0),
_refCount(0)
#else
_refMutex(0),
_refCount(0),
_observers(0)
_observerSet(0)
#endif
{
#if !defined(_OSG_REFERENCED_USE_ATOMIC_OPERATIONS)
@ -161,7 +159,7 @@ Referenced::Referenced():
{
OpenThreads::ScopedLock<OpenThreads::Mutex> lock(getNumObjectMutex());
++s_numObjects;
osg::notify(osg::NOTICE)<<"Object created, total num="<<s_numObjects<<std::endl;
OSG_NOTICE<<"Object created, total num="<<s_numObjects<<std::endl;
}
#endif
@ -169,12 +167,12 @@ Referenced::Referenced():
Referenced::Referenced(bool threadSafeRefUnref):
#if defined(_OSG_REFERENCED_USE_ATOMIC_OPERATIONS)
_observerSetDataPtr(0),
_observerSet(0),
_refCount(0)
#else
_refMutex(0),
_refCount(0),
_observers(0)
_observerSet(0)
#endif
{
#if !defined(_OSG_REFERENCED_USE_ATOMIC_OPERATIONS)
@ -188,19 +186,19 @@ Referenced::Referenced(bool threadSafeRefUnref):
{
OpenThreads::ScopedLock<OpenThreads::Mutex> lock(getNumObjectMutex());
++s_numObjects;
osg::notify(osg::NOTICE)<<"Object created, total num="<<s_numObjects<<std::endl;
OSG_NOTICE<<"Object created, total num="<<s_numObjects<<std::endl;
}
#endif
}
Referenced::Referenced(const Referenced&):
#if defined(_OSG_REFERENCED_USE_ATOMIC_OPERATIONS)
_observerSetDataPtr(0),
_observerSet(0),
_refCount(0)
#else
_refMutex(0),
_refCount(0),
_observers(0)
_observerSet(0)
#endif
{
#if !defined(_OSG_REFERENCED_USE_ATOMIC_OPERATIONS)
@ -214,7 +212,7 @@ Referenced::Referenced(const Referenced&):
{
OpenThreads::ScopedLock<OpenThreads::Mutex> lock(getNumObjectMutex());
++s_numObjects;
osg::notify(osg::NOTICE)<<"Object created, total num="<<s_numObjects<<std::endl;
OSG_NOTICE<<"Object created, total num="<<s_numObjects<<std::endl;
}
#endif
}
@ -225,129 +223,84 @@ Referenced::~Referenced()
{
OpenThreads::ScopedLock<OpenThreads::Mutex> lock(getNumObjectMutex());
--s_numObjects;
osg::notify(osg::NOTICE)<<"Object deleted, total num="<<s_numObjects<<std::endl;
OSG_NOTICE<<"Object deleted, total num="<<s_numObjects<<std::endl;
}
#endif
if (_refCount>0)
{
notify(WARN)<<"Warning: deleting still referenced object "<<this<<" of type '"<<typeid(this).name()<<"'"<<std::endl;
notify(WARN)<<" the final reference count was "<<_refCount<<", memory corruption possible."<<std::endl;
OSG_WARN<<"Warning: deleting still referenced object "<<this<<" of type '"<<typeid(this).name()<<"'"<<std::endl;
OSG_WARN<<" the final reference count was "<<_refCount<<", memory corruption possible."<<std::endl;
}
// signal observers that we are being deleted.
signalObserversAndDelete(false, true, false);
// delete the ObserverSet
#if defined(_OSG_REFERENCED_USE_ATOMIC_OPERATIONS)
if (_observerSet.get()) delete static_cast<ObserverSet*>(_observerSet.get());
#else
if (_observerSet) delete static_cast<ObserverSet*>(_observerSet);
#endif
}
ObserverSet* Referenced::getOrCreateObserverSet() const
{
#if defined(_OSG_REFERENCED_USE_ATOMIC_OPERATIONS)
ObserverSet* observerSet = static_cast<ObserverSet*>(_observerSet.get());
while (0 == observerSet) {
ObserverSet* newObserverSet = new ObserverSet;
if (!_observerSet.assign(newObserverSet, 0))
delete newObserverSet;
observerSet = static_cast<ObserverSet*>(_observerSet.get());
}
return observerSet;
#else
if (_refMutex)
{
OpenThreads::ScopedLock<OpenThreads::Mutex> lock(*_refMutex);
if (!_observerSet) _observerSet = new ObserverSet;
return static_cast<ObserverSet*>(_observerSet);
}
else
{
if (!_observerSet) _observerSet = new ObserverSet;
return static_cast<ObserverSet*>(_observerSet);
}
#endif
}
void Referenced::signalObserversAndDelete(bool signalUnreferened, bool signalDelete, bool doDelete) const
{
#if defined(_OSG_REFERENCED_USE_ATOMIC_OPERATIONS)
ObserverSet* observerSet = static_cast<ObserverSet*>(_observerSet.get());
#else
ObserverSet* observerSet = static_cast<ObserverSet*>(_observerSet);
#endif
if (observerSet)
{
OpenThreads::ScopedLock<OpenThreads::Mutex> lock(*(observerSet->getObserverSetMutex()));
if (signalUnreferened)
{
// tell all observers that we have been unreferenced so that they
// can do clean up or add their own reference to prevent deletion.
#if !defined(_OSG_REFERENCED_USE_ATOMIC_OPERATIONS)
ObserverSet* os = static_cast<ObserverSet*>(_observers);
if (os)
{
if (_refMutex)
{
OpenThreads::ScopedLock<OpenThreads::Mutex> lock(*_refMutex);
for(ObserverSet::iterator itr = os->begin();
itr != os->end();
)
{
if ((*itr)->objectUnreferenced(const_cast<Referenced*>(this)))
{
ObserverSet::iterator orig_itr = itr;
++itr;
os.erase(orig_itr);
}
else
{
++itr;
}
}
}
else
{
for(ObserverSet::iterator itr = os->begin();
itr != os->end();
)
{
if ((*itr)->objectUnreferenced(const_cast<Referenced*>(this)))
{
ObserverSet::iterator orig_itr = itr;
++itr;
os.erase(orig_itr);
}
else
{
++itr;
}
}
}
}
#else
ObserverSetData* observerSetData = static_cast<ObserverSetData*>(_observerSetDataPtr.get());
if (observerSetData)
{
OpenThreads::ScopedLock<OpenThreads::Mutex> lock(observerSetData->_mutex);
for(ObserverSet::iterator itr = observerSetData->_observers.begin();
itr != observerSetData->_observers.end();
)
{
if ((*itr)->objectUnreferenced(const_cast<Referenced*>(this)))
{
ObserverSet::iterator orig_itr = itr;
++itr;
observerSetData->_observers.erase(orig_itr);
}
else
{
++itr;
}
}
}
#endif
observerSet->signalObjectUnreferenced(const_cast<Referenced*>(this));
}
if (_refCount!=0) return;
if (_refCount!=0)
{
OSG_NOTICE<<"Referenced::signalObserversAndDelete(,,) disabling delete as _refCount="<<_refCount<<std::endl;
return;
}
if (signalDelete)
{
// tell all observers that we being delete so that they
// can do cleans up and remove any references they have.
#if !defined(_OSG_REFERENCED_USE_ATOMIC_OPERATIONS)
ObserverSet* os = static_cast<ObserverSet*>(_observers);
if (os)
{
for(ObserverSet::iterator itr = os->begin();
itr != os->end();
++itr)
{
(*itr)->objectDeleted(const_cast<Referenced*>(this));
observerSet->signalObjectDeleted(const_cast<Referenced*>(this));
}
delete os;
_observers = 0;
}
#else
ObserverSetData* observerSetData = static_cast<ObserverSetData*>(_observerSetDataPtr.get());
if (observerSetData)
{
for(ObserverSet::iterator itr = observerSetData->_observers.begin();
itr != observerSetData->_observers.end();
++itr)
{
(*itr)->objectDeleted(const_cast<Referenced*>(this));
}
_observerSetDataPtr.assign(0, observerSetData);
delete observerSetData;
}
#endif
}
if (doDelete &&_refCount<=0)
if (doDelete && _refCount==0)
{
//OSG_NOTICE<<"Referenced::signalObserversAndDelete(,,) doing delete as _refCount="<<_refCount<<std::endl;
if (getDeleteHandler()) deleteUsingDeleteHandler();
else delete this;
}
@ -378,22 +331,21 @@ void Referenced::setThreadSafeRefUnref(bool threadSafe)
#endif
}
void Referenced::unref_nodelete() const
{
#if !defined(_OSG_REFERENCED_USE_ATOMIC_OPERATIONS)
#if defined(_OSG_REFERENCED_USE_ATOMIC_OPERATIONS)
bool needUnreferencedSignal = ((--_refCount) == 0);
#else
bool needUnreferencedSignal = false;
if (_refMutex)
{
OpenThreads::ScopedLock<OpenThreads::Mutex> lock(*_refMutex);
needUnreferencedSignal = (--_refCount)<=0;
needUnreferencedSignal = ((--_refCount) == 0);
}
else
{
needUnreferencedSignal = (--_refCount)<=0;
needUnreferencedSignal = ((--_refCount) == 0);
}
#else
bool needUnreferencedSignal = (--_refCount)<=0;
#endif
if (needUnreferencedSignal)
@ -402,57 +354,6 @@ void Referenced::unref_nodelete() const
}
}
void Referenced::addObserver(Observer* observer) const
{
#if defined(_OSG_REFERENCED_USE_ATOMIC_OPERATIONS)
ObserverSetData* observerSetData = static_cast<ObserverSetData*>(_observerSetDataPtr.get());
while (0 == observerSetData) {
ObserverSetData* newObserverSetData = new ObserverSetData;
if (!_observerSetDataPtr.assign(newObserverSetData, 0))
delete newObserverSetData;
observerSetData = static_cast<ObserverSetData*>(_observerSetDataPtr.get());
}
OpenThreads::ScopedLock<OpenThreads::Mutex> lock(observerSetData->_mutex);
observerSetData->_observers.insert(observer);
#else
if (_refMutex)
{
OpenThreads::ScopedLock<OpenThreads::Mutex> lock(*_refMutex);
if (!_observers) _observers = new ObserverSet;
if (_observers) static_cast<ObserverSet*>(_observers)->insert(observer);
}
else
{
if (!_observers) _observers = new ObserverSet;
if (_observers) static_cast<ObserverSet*>(_observers)->insert(observer);
}
#endif
}
void Referenced::removeObserver(Observer* observer) const
{
#if defined(_OSG_REFERENCED_USE_ATOMIC_OPERATIONS)
ObserverSetData* observerSetData = static_cast<ObserverSetData*>(_observerSetDataPtr.get());
if (observerSetData)
{
OpenThreads::ScopedLock<OpenThreads::Mutex> lock(observerSetData->_mutex);
observerSetData->_observers.erase(observer);
}
#else
if (_refMutex)
{
OpenThreads::ScopedLock<OpenThreads::Mutex> lock(*_refMutex);
if (_observers) static_cast<ObserverSet*>(_observers)->erase(observer);
}
else
{
if (_observers) static_cast<ObserverSet*>(_observers)->erase(observer);
}
#endif
}
void Referenced::deleteUsingDeleteHandler() const
{
getDeleteHandler()->requestDelete(this);