diff --git a/panda/src/express/referenceCount.I b/panda/src/express/referenceCount.I index 3a05a68766..f18c60a02c 100644 --- a/panda/src/express/referenceCount.I +++ b/panda/src/express/referenceCount.I @@ -300,6 +300,27 @@ weak_unref() { nassertv(nonzero); } +/** + * Atomically increases the reference count of this object if it is not zero. + * Do not use this. This exists only to implement a special case for weak + * pointers. + * @return true if the reference count was incremented, false if it was zero. + */ +INLINE bool ReferenceCount:: +ref_if_nonzero() const { +#ifdef _DEBUG + test_ref_count_integrity(); +#endif + AtomicAdjust::Integer ref_count; + do { + ref_count = AtomicAdjust::get(_ref_count); + if (ref_count <= 0) { + return false; + } + } while (ref_count != AtomicAdjust::compare_and_exchange(_ref_count, ref_count, ref_count + 1)); + return true; +} + /** * This global helper function will unref the given ReferenceCount object, and * if the reference count reaches zero, automatically delete it. It can't be diff --git a/panda/src/express/referenceCount.h b/panda/src/express/referenceCount.h index 2260cf2e8c..ba27ac7db8 100644 --- a/panda/src/express/referenceCount.h +++ b/panda/src/express/referenceCount.h @@ -63,6 +63,8 @@ public: INLINE WeakReferenceList *weak_ref(); INLINE void weak_unref(); + INLINE bool ref_if_nonzero() const; + protected: bool do_test_ref_count_integrity() const; bool do_test_ref_count_nonzero() const; diff --git a/panda/src/express/weakPointerTo.I b/panda/src/express/weakPointerTo.I index 09dba7640b..60ac8a1917 100644 --- a/panda/src/express/weakPointerTo.I +++ b/panda/src/express/weakPointerTo.I @@ -75,6 +75,9 @@ operator T * () const { /** * A thread-safe way to access the underlying pointer; will silently return * null if the underlying pointer was deleted or null. + * Note that this may return null even if was_deleted() still returns true, + * which can occur if the object has reached reference count 0 and is about to + * be destroyed. */ template INLINE PointerTo WeakPointerTo:: @@ -84,7 +87,13 @@ lock() const { PointerTo ptr; weak_ref->_lock.lock(); if (!weak_ref->was_deleted()) { - ptr = (To *)WeakPointerToBase::_void_ptr; + // We also need to check that the reference count is not zero (which can + // happen if the object is currently being destructed), since that could + // cause double deletion. + To *plain_ptr = (To *)WeakPointerToBase::_void_ptr; + if (plain_ptr != nullptr && plain_ptr->ref_if_nonzero()) { + ptr.cheat() = plain_ptr; + } } weak_ref->_lock.unlock(); return ptr; @@ -239,7 +248,13 @@ lock() const { ConstPointerTo ptr; weak_ref->_lock.lock(); if (!weak_ref->was_deleted()) { - ptr = (const To *)WeakPointerToBase::_void_ptr; + // We also need to check that the reference count is not zero (which can + // happen if the object is currently being destructed), since that could + // cause double deletion. + const To *plain_ptr = (const To *)WeakPointerToBase::_void_ptr; + if (plain_ptr != nullptr && plain_ptr->ref_if_nonzero()) { + ptr.cheat() = plain_ptr; + } } weak_ref->_lock.unlock(); return ptr;