diff --git a/include/osg/Referenced b/include/osg/Referenced index 4fb34060b..452e0386a 100644 --- a/include/osg/Referenced +++ b/include/osg/Referenced @@ -89,8 +89,7 @@ class OSG_EXPORT Referenced not delete it, even if ref count goes to 0. Warning, unref_nodelete() 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. - */ + as the later can lead to memory leaks.*/ int unref_nodelete() const; /** Return the number pointers currently referencing this object. */ diff --git a/include/osg/observer_ptr b/include/osg/observer_ptr index b8dcc1542..498b3e5c0 100644 --- a/include/osg/observer_ptr +++ b/include/osg/observer_ptr @@ -72,23 +72,31 @@ struct UnsafeWeakReference : public WeakReference } }; /** - * Delete the WeakReference object if necessary while negociating - * with the observer in the referenced object. - */ + Delete the WeakReference object, if necessary. The WeakReference + should be deleted when the last observer_ptr pointing to it goes + out of scope; this avoids a memory leak. A race must be prevented + between this function and WeakReference::objectDeleted(). If the + referenced object is being deleted at the same time as this + function is called, we can't prevent objectDeleted() from being + run, so we need to leave the WeakReference in a valid state and let + objectDeleted() delete it. */ template void maybeDelete(WeakReference* weakRef) { if (!weakRef || weakRef->unref_nodelete() > 0) return; + // No other observer_ptrs hold weakRef, so clean up. bool doDelete = false; { OpenThreads::ScopedLock lock(weakRef->_mutex); if (!weakRef->_ptr) { + // The object has already been deleted; it's OK to delete weakRef. doDelete = true; } else { + // Carefully remove weakRef as an observer. UnsafeWeakReference* unsafe = dynamic_cast*>(weakRef); if (!unsafe && weakRef->_ptr->ref() == 1) @@ -100,6 +108,9 @@ void maybeDelete(WeakReference* weakRef) } else { + // The referenced object won't be deleted until we + // decrement the reference count, so go ahead and + // remove the observer. doDelete = true; weakRef->_ptr->removeObserver(weakRef); if (!unsafe)