Acquire mutex in cache_unref() - fixes race cond in async flatten

This commit is contained in:
rdb 2016-09-11 22:14:18 +02:00
parent 3ce808c705
commit 632ad5e3ef
3 changed files with 37 additions and 6 deletions

View File

@ -83,6 +83,17 @@ cache_ref() const {
MutexHolder holder(_lock_mutex); MutexHolder holder(_lock_mutex);
CachedTypedWritableReferenceCount::cache_ref(); CachedTypedWritableReferenceCount::cache_ref();
} }
////////////////////////////////////////////////////////////////////
// Function: CopyOnWriteObject::cache_unref
// Access: Published
// Description: See CachedTypedWritableReferenceCount::cache_unref().
////////////////////////////////////////////////////////////////////
INLINE bool CopyOnWriteObject::
cache_unref() const {
MutexHolder holder(_lock_mutex);
return CachedTypedWritableReferenceCount::cache_unref();
}
#endif // COW_THREADED #endif // COW_THREADED
//////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////

View File

@ -51,6 +51,7 @@ PUBLISHED:
#ifdef COW_THREADED #ifdef COW_THREADED
virtual bool unref() const; virtual bool unref() const;
INLINE void cache_ref() const; INLINE void cache_ref() const;
INLINE bool cache_unref() const;
#endif // COW_THREADED #endif // COW_THREADED
protected: protected:

View File

@ -13,7 +13,6 @@
//////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////
#include "copyOnWritePointer.h" #include "copyOnWritePointer.h"
#include "mutexHolder.h"
#include "config_util.h" #include "config_util.h"
#include "config_pipeline.h" #include "config_pipeline.h"
@ -78,7 +77,7 @@ get_write_pointer() {
Thread *current_thread = Thread::get_current_thread(); Thread *current_thread = Thread::get_current_thread();
MutexHolder holder(_cow_object->_lock_mutex); _cow_object->_lock_mutex.acquire();
while (_cow_object->_lock_status == CopyOnWriteObject::LS_locked_write && while (_cow_object->_lock_status == CopyOnWriteObject::LS_locked_write &&
_cow_object->_locking_thread != current_thread) { _cow_object->_locking_thread != current_thread) {
if (util_cat.is_debug()) { if (util_cat.is_debug()) {
@ -98,10 +97,20 @@ get_write_pointer() {
<< "Making copy of " << _cow_object->get_type() << "Making copy of " << _cow_object->get_type()
<< " because it is locked in read mode.\n"; << " because it is locked in read mode.\n";
} }
PT(CopyOnWriteObject) new_object = _cow_object->make_cow_copy(); PT(CopyOnWriteObject) new_object = _cow_object->make_cow_copy();
cache_unref_delete(_cow_object);
// We can't call cache_unref_delete, because we hold the lock.
if (!_cow_object->CachedTypedWritableReferenceCount::cache_unref()) {
_cow_object->_lock_mutex.release();
delete _cow_object;
} else {
_cow_object->_lock_mutex.release();
}
_cow_object = new_object; _cow_object = new_object;
_cow_object->cache_ref(); _cow_object->cache_ref();
_cow_object->_lock_status = CopyOnWriteObject::LS_locked_write;
_cow_object->_locking_thread = current_thread;
} else if (_cow_object->get_cache_ref_count() > 1) { } else if (_cow_object->get_cache_ref_count() > 1) {
// No one else has it specifically read-locked, but there are // No one else has it specifically read-locked, but there are
@ -115,9 +124,18 @@ get_write_pointer() {
} }
PT(CopyOnWriteObject) new_object = _cow_object->make_cow_copy(); PT(CopyOnWriteObject) new_object = _cow_object->make_cow_copy();
cache_unref_delete(_cow_object);
// We can't call cache_unref_delete, because we hold the lock.
if (!_cow_object->CachedTypedWritableReferenceCount::cache_unref()) {
_cow_object->_lock_mutex.release();
delete _cow_object;
} else {
_cow_object->_lock_mutex.release();
}
_cow_object = new_object; _cow_object = new_object;
_cow_object->cache_ref(); _cow_object->cache_ref();
_cow_object->_lock_status = CopyOnWriteObject::LS_locked_write;
_cow_object->_locking_thread = current_thread;
} else { } else {
// No other thread has the pointer locked, and we're the only // No other thread has the pointer locked, and we're the only
@ -127,9 +145,10 @@ get_write_pointer() {
// We can't assert that there are no outstanding ordinary // We can't assert that there are no outstanding ordinary
// references to it, though, since the creator of the object might // references to it, though, since the creator of the object might
// have saved himself a reference. // have saved himself a reference.
}
_cow_object->_lock_status = CopyOnWriteObject::LS_locked_write; _cow_object->_lock_status = CopyOnWriteObject::LS_locked_write;
_cow_object->_locking_thread = current_thread; _cow_object->_locking_thread = current_thread;
_cow_object->_lock_mutex.release();
}
return _cow_object; return _cow_object;
} }