From 2b2ea4487a8cc7fcd21f90ecb6ec1546087dba4b Mon Sep 17 00:00:00 2001 From: Robert Osfield Date: Fri, 14 May 2010 12:14:14 +0000 Subject: [PATCH] From Tim Moore, new more robust observer_ptr<> implementation --- include/osg/Referenced | 27 +-- include/osg/observer_ptr | 350 +++++++++++++++++++++++++++++---------- src/osg/Referenced.cpp | 32 ++-- 3 files changed, 295 insertions(+), 114 deletions(-) diff --git a/include/osg/Referenced b/include/osg/Referenced index ad6fe2743..848eef3a5 100644 --- a/include/osg/Referenced +++ b/include/osg/Referenced @@ -76,13 +76,13 @@ class OSG_EXPORT Referenced /** Increment the reference count by one, indicating that this object has another pointer which is referencing it.*/ - inline void ref() const; + inline int ref() const; /** Decrement the reference count by one, indicating that a pointer to this object is referencing it. If the reference count goes to zero, it is assumed that this object is no longer referenced and is automatically deleted.*/ - inline void unref() const; + inline int unref() const; /** Decrement the reference count by one, indicating that a pointer to this object is referencing it. However, do @@ -90,7 +90,7 @@ class OSG_EXPORT Referenced should only be called if the user knows exactly who will be responsible for, one should prefer unref() over unref_nodelete() as the later can lead to memory leaks.*/ - void unref_nodelete() const; + int unref_nodelete() const; /** Return the number pointers currently referencing this object. */ inline int referenceCount() const { return _refCount; } @@ -155,37 +155,41 @@ class OSG_EXPORT Referenced #endif }; -inline void Referenced::ref() const +inline int Referenced::ref() const { #if defined(_OSG_REFERENCED_USE_ATOMIC_OPERATIONS) - ++_refCount; + return ++_refCount; #else if (_refMutex) { OpenThreads::ScopedLock lock(*_refMutex); - ++_refCount; + return ++_refCount; } else { - ++_refCount; + return ++_refCount; } #endif } -inline void Referenced::unref() const +inline int Referenced::unref() const { + int newRef; #if defined(_OSG_REFERENCED_USE_ATOMIC_OPERATIONS) - bool needDelete = ((--_refCount) == 0); + newRef = --_refCount; + bool needDelete = (newRef == 0); #else bool needDelete = false; if (_refMutex) { OpenThreads::ScopedLock lock(*_refMutex); - needDelete = (--_refCount)==0; + newRef = --_refCount; + needDelete = newRef==0; } else { - needDelete = (--_refCount)==0; + newRef = --_refCount; + needDelete = newRef==0; } #endif @@ -193,6 +197,7 @@ inline void Referenced::unref() const { signalObserversAndDelete(true,true,true); } + return newRef; } // intrusive_ptr_add_ref and intrusive_ptr_release allow diff --git a/include/osg/observer_ptr b/include/osg/observer_ptr index e0e3362c9..b8dcc1542 100644 --- a/include/osg/observer_ptr +++ b/include/osg/observer_ptr @@ -18,10 +18,108 @@ #include #include +#include +#include + namespace osg { +// Internal class used to hold the "naked" pointer to the weakly +// referenced object +template +struct WeakReference : public Observer, public Referenced +{ + WeakReference(const T* ptr) : _ptr(const_cast(ptr)) {} + + void objectDeleted(void*); + /** + * "Lock" a Referenced object i.e., protect it from being deleted + * by incrementing its reference count. + * + * returns null if object doesn't exist anymore + */ + virtual T* addRefLock() + { + OpenThreads::ScopedLock lock(_mutex); + if (!_ptr) return 0; + int refCount = _ptr->ref(); + if (refCount == 1) + { + // The object is in the process of being deleted, but our + // objectDeleted() method hasn't been run yet (and we're + // blocking it -- and the final destruction -- with our lock). + _ptr->unref_nodelete(); + return 0; + } + return _ptr; + } + OpenThreads::Mutex _mutex; + T* _ptr; +}; + +// Reference object for observer_ptr that was initialized with an +// "unsafe" object e.g., a stack-allocated object. + +template +struct UnsafeWeakReference : public WeakReference +{ + UnsafeWeakReference(const T* ptr) : WeakReference(ptr) {} + + // Don't even touch the ref count of an unsafe object + T* addRefLock() + { + OSG_WARN << "tried to lock an unsafe observer_ptr."; + return 0; + } +}; +/** + * Delete the WeakReference object if necessary while negociating + * with the observer in the referenced object. + */ +template +void maybeDelete(WeakReference* weakRef) +{ + if (!weakRef || weakRef->unref_nodelete() > 0) + return; + bool doDelete = false; + { + OpenThreads::ScopedLock lock(weakRef->_mutex); + if (!weakRef->_ptr) + { + doDelete = true; + } + else + { + UnsafeWeakReference* unsafe + = dynamic_cast*>(weakRef); + if (!unsafe && weakRef->_ptr->ref() == 1) + { + // The referenced object is being deleted, so the + // Observer method must delete the weak reference. + weakRef->_ptr->unref_nodelete(); + weakRef->_ptr = 0; + } + else + { + doDelete = true; + weakRef->_ptr->removeObserver(weakRef); + if (!unsafe) + weakRef->_ptr->unref_nodelete(); + } + } + } + if (doDelete) delete weakRef; +} + +/** + * Get a weak reference from a Referenced. This returns a + * WeakReference object with the ref count bumped. We must already + * hold a reference to the Referenced object so it won't get + * deleted during this process. + */ +template WeakReference* getWeakReference(const T* object); + /** Smart pointer for observed objects, that automatically set pointers to them to null when they deleted. - * To use the observer_ptr<> robustly in mulit-threaded applications it is recommend to access the pointer via + * To use the observer_ptr<> robustly in multi-threaded applications it is recommend to access the pointer via * the lock() method that passes back a ref_ptr<> that safely takes a reference to the object to prevent deletion * during usage of the object. In certain conditions it may be safe to use the pointer directly without using lock(), * which will confer a perfomance advantage, the conditions are: @@ -34,96 +132,182 @@ namespace osg { template class observer_ptr : public Observer { +public: + typedef T element_type; + observer_ptr() : _reference(0) {} + /** + * Create a observer_ptr from a ref_ptr. + */ + observer_ptr(ref_ptr& rp) + { + _reference = getWeakReference(rp.get()); + } + /** + * Create a observer_ptr from a raw pointer. For compatibility; + * the result might not be lockable. + */ + observer_ptr(T* rp) + { + _reference = getWeakReference(rp); + } + + observer_ptr(const observer_ptr& wp) : _reference(wp._reference) + { + if (_reference) _reference->ref(); + } - public: - typedef T element_type; + ~observer_ptr() + { + maybeDelete(_reference); + } + + observer_ptr& operator = (const observer_ptr& wp) + { + if (wp._reference) wp._reference->ref(); + maybeDelete(_reference); + _reference = wp._reference; + return *this; + } - observer_ptr(): _ptr(0L) {} - observer_ptr(T* t): _ptr(t) { if (_ptr) _ptr->addObserver(this); } - observer_ptr(const observer_ptr& rp): - Observer() + observer_ptr& operator = (const ref_ptr& rp) + { + WeakReference* tmp = getWeakReference(rp.get()); + maybeDelete(_reference); + _reference = tmp; + return *this; + } + + observer_ptr& operator = (T* rp) + { + WeakReference* tmp = getWeakReference(rp); + maybeDelete(_reference); + _reference = tmp; + return *this; + } + /** + * Create a ref_ptr from a observer_ptr. The ref_ptr will be valid if the + * referenced object hasn't been deleted and has a ref count > 0. + */ + ref_ptr lock() const + { + if (!_reference) + return ref_ptr(); + T* obj = _reference->addRefLock(); + if (!obj) + return ref_ptr(); + ref_ptr result(obj); + obj->unref_nodelete(); + return result; + } + /** Comparison operators. These continue to work even after the + * observed object has been deleted. + */ + bool operator == (const observer_ptr& wp) const + { return _reference == wp._reference; } + bool operator != (const observer_ptr& wp) const + { return _reference != wp._reference; } + bool operator < (const observer_ptr& wp) const + { return _reference < wp._reference; } + bool operator > (const observer_ptr& wp) const + { return _reference < wp._reference; } + // Non-strict interface, for compatibility + // comparison operator for const T*. + inline bool operator == (const T* ptr) const + { + if (!_reference) + return !ptr; + else + return (_reference->_ptr==ptr); + } + inline bool operator != (const T* ptr) const + { + if (!_reference) + return ptr; + else + return (_reference->_ptr!=ptr); + } + inline bool operator < (const T* ptr) const { return (_reference && _reference->_ptr (const T* ptr) const { return (_reference && _reference->ptr>ptr); } + // Convenience methods for operating on object, however, access is not automatically threadsafe. + // To make thread safe, one should either ensure at a high level + // that the object will not be deleted while operating on it, or + // by using the observer_ptr<>::lock() to get a ref_ptr<> that + // ensures the objects stay alive throughout all access to it. + + // Throw an error if _reference is null? + inline T& operator*() const { return *_reference->_ptr; } + inline T* operator->() const { return _reference? _reference->_ptr : 0; } + // get the raw C pointer + inline T* get() const { return _reference ? _reference->_ptr : 0; } + + inline bool operator!() const { return !_reference || !_reference->_ptr; } + inline bool valid() const { return _reference && _reference->_ptr; } +protected: + // The pointer to the WeakReference is not kept in a ref_ptr so that + // its deletion can be managed explicitly. + WeakReference* _reference; +}; + +template +void WeakReference::objectDeleted(void*) +{ + bool deleteNeeded = false; + { + OpenThreads::ScopedLock lock(_mutex); + if (!_ptr) { - OpenThreads::ScopedLock lock(*getObserverMutex()); - _ptr = rp._ptr; - if (_ptr) _ptr->addObserver(this); + // The last weak reference was deleted after the last + // reference, but the observer hasn't run yet. The + // observer can't be prevented from running, so it + // must delete itself. + deleteNeeded = true; } - - ~observer_ptr() + else { - OpenThreads::ScopedLock lock(*getObserverMutex()); - if (_ptr) _ptr->removeObserver(this); - } - - inline observer_ptr& operator = (const observer_ptr& rp) - { - if (&rp==this) return *this; - - OpenThreads::ScopedLock lock(*getObserverMutex()); - - if (_ptr==rp._ptr) return *this; - if (_ptr) _ptr->removeObserver(this); - - _ptr = rp._ptr; - if (_ptr) _ptr->addObserver(this); - return *this; - } - - inline observer_ptr& operator = (T* ptr) - { - OpenThreads::ScopedLock lock(*getObserverMutex()); - - if (_ptr==ptr) return *this; - if (_ptr) _ptr->removeObserver(this); - - _ptr = ptr; - if (_ptr) _ptr->addObserver(this); - - return *this; - } - - // robust thread safe access to pointer - ref_ptr lock() const - { - OpenThreads::ScopedLock lock(*getObserverMutex()); - if (_ptr && _ptr->referenceCount()>0) return ref_ptr(_ptr); - else return ref_ptr(_ptr); - } - - // get the raw C pointer - inline T* get() const { return _ptr; } - - // comparison operators for observer_ptr. - inline bool operator == (const observer_ptr& rp) const { return (_ptr==rp._ptr); } - inline bool operator != (const observer_ptr& rp) const { return (_ptr!=rp._ptr); } - inline bool operator < (const observer_ptr& rp) const { return (_ptr (const observer_ptr& rp) const { return (_ptr>rp._ptr); } - - // comparison operator for const T*. - inline bool operator == (const T* ptr) const { return (_ptr==ptr); } - inline bool operator != (const T* ptr) const { return (_ptr!=ptr); } - inline bool operator < (const T* ptr) const { return (_ptr (const T* ptr) const { return (_ptr>ptr); } - - // convinience methods for operating on object, however, access to not automatically threadsafe - // to make thread safe one should either ensure a high level that object will not be deleted - // which operating on it, or by using the observer_ptr<>::lock() to get a ref_ptr<> that ensures the - // objects stay alive throughout all access to it. - inline T& operator*() const { return *_ptr; } - inline T* operator->() const { return _ptr; } - - inline bool operator!() const { return _ptr==0L; } - inline bool valid() const { return _ptr!=0L; } - - protected: - - virtual void objectDeleted(void*) - { - OpenThreads::ScopedLock lock(*getObserverMutex()); _ptr = 0; } + } + if (deleteNeeded) delete this; +} - T* _ptr; -}; +template +WeakReference* +getWeakReference(const T* object) +{ + if (!object) + return 0; + WeakReference* result = 0; + ObserverSet* setData = object->getOrCreateObserverSet(); + OpenThreads::ScopedLock lock(*setData->getObserverSetMutex()); + ObserverSet::Observers &observers = setData->getObservers(); + for (ObserverSet::Observers::iterator itr = observers.begin(), + end = observers.end(); + itr != end; + ++itr) + { + if ((result = dynamic_cast*>(*itr))) + { + int weakRefCount = result->ref(); + if (weakRefCount == 1) + { + // The last weak reference disappeared, but hasn't + // been removed from the observers list yet. + result->unref_nodelete(); + } + else + { + return result; + } + } + } + if (object->referenceCount() > 0) + result = new WeakReference(object); + else + result = new UnsafeWeakReference(object); + result->ref(); + observers.insert(result); + return result; +} } diff --git a/src/osg/Referenced.cpp b/src/osg/Referenced.cpp index 269e23e6d..b627f8cf2 100644 --- a/src/osg/Referenced.cpp +++ b/src/osg/Referenced.cpp @@ -87,20 +87,12 @@ OpenThreads::Mutex* Referenced::getGlobalReferencedMutex() return s_ReferencedGlobalMutext.get(); } -// 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(); -} - // helper class for forcing the global mutex to be constructed when the library is loaded. struct InitGlobalMutexes { InitGlobalMutexes() { Referenced::getGlobalReferencedMutex(); - Observer::getGlobalObserverMutex(); } }; static InitGlobalMutexes s_initGlobalMutexes; @@ -302,21 +294,16 @@ void Referenced::signalObserversAndDelete(bool signalUnreferened, bool signalDel observerSet->signalObjectUnreferenced(const_cast(this)); } - if (_refCount!=0) - { - OSG_NOTICE<<"Referenced::signalObserversAndDelete(,,) disabling delete as _refCount="<<_refCount<signalObjectDeleted(const_cast(this)); } } - if (doDelete && _refCount==0) + if (doDelete) { - //OSG_NOTICE<<"Referenced::signalObserversAndDelete(,,) doing delete as _refCount="<<_refCount<