From cac09ba702800ff6981c3c1958e2bc5c103a5c7f Mon Sep 17 00:00:00 2001 From: Xavier Thompson <xavier.thompson@nexedi.com> Date: Thu, 5 Nov 2020 19:18:17 +0100 Subject: [PATCH] Reimplement cypclass lock using nonrecursive pthread_rwlock_t --- Cython/Utility/CyObjects.cpp | 258 ++--------------------------------- 1 file changed, 11 insertions(+), 247 deletions(-) diff --git a/Cython/Utility/CyObjects.cpp b/Cython/Utility/CyObjects.cpp index 08b33ea7a..477bab46a 100644 --- a/Cython/Utility/CyObjects.cpp +++ b/Cython/Utility/CyObjects.cpp @@ -16,13 +16,6 @@ #include <atomic> using namespace std; #define CyObject_ATOMIC_REFCOUNT_TYPE atomic_int - #define CyObject_NO_OWNER -1 - #define CyObject_MANY_OWNERS -2 - - #define CyObject_CONTENDING_WRITER_FLAG (1 << 0) - #define CyObject_CONTENDING_READER_FLAG (1 << 1) - - #define CyObject_RAISE_ON_CONTENTION 0 #include <pthread.h> @@ -40,28 +33,14 @@ class CyLock { - static pthread_mutex_t log_guard; protected: - pthread_mutex_t guard; - pthread_cond_t readers_have_left; - pthread_cond_t writer_has_left; - atomic<pid_t> owner_id; - atomic_int readers_nb; - uint32_t write_count; - const char *owner_context; + pthread_rwlock_t rwlock; public: CyLock() { - pthread_mutex_init(&this->guard, NULL); - pthread_cond_init(&this->readers_have_left, NULL); - pthread_cond_init(&this->writer_has_left, NULL); - this->owner_id = CyObject_NO_OWNER; - this->readers_nb = 0; - this->write_count = 0; + pthread_rwlock_init(&this->rwlock, NULL); } ~CyLock() { - pthread_mutex_destroy(&this->guard); - pthread_cond_destroy(&this->readers_have_left); - pthread_cond_destroy(&this->writer_has_left); + pthread_rwlock_destroy(&this->rwlock); } void wlock(const char * context); void rlock(const char * context); @@ -70,9 +49,6 @@ int tryrlock(); int trywlock(); }; - #if CyObject_RAISE_ON_CONTENTION == 0 - pthread_mutex_t CyLock::log_guard = PTHREAD_MUTEX_INITIALIZER; - #endif struct CyPyObject { PyObject_HEAD @@ -541,240 +517,28 @@ #endif /* __cplusplus */ -void CyLock::rlock(const char *context) { - pid_t caller_id = syscall(SYS_gettid); - - if (this->owner_id == caller_id) { - ++this->readers_nb; - return; - } - - pthread_mutex_lock(&this->guard); - - if (this->write_count > 0) { - #if CyObject_RAISE_ON_CONTENTION - pid_t owner_id = this->owner_id; - std::ostringstream msg; - msg << "Data Race between [this] reader #" << caller_id - << " and [other] writer #" << owner_id - << " on lock " << this; - if (context != NULL) { - msg << std::endl << "In [this] context: " << context; - } - if (this->owner_context != NULL) { - msg << std::endl << "In [other] context: " << this->owner_context; - } - throw std::runtime_error(msg.str()); - #else - pid_t owner_id = this->owner_id; - pthread_mutex_lock(&(CyLock::log_guard)); - std::cerr - << "Data Race between [this] reader #" << caller_id - << " and [other] writer #" << owner_id - << " on lock " << this << std::endl; - if (context != NULL) { - std::cerr << "In [this] context: " << context << std::endl; - } - if (this->owner_context != NULL) { - std::cerr << "In [other] context: " << this->owner_context << std::endl; - } - pthread_mutex_unlock(&(CyLock::log_guard)); - #endif - } - - while (this->write_count > 0) { - pthread_cond_wait(&this->writer_has_left, &this->guard); - } - - this->owner_id = this->readers_nb++ ? CyObject_MANY_OWNERS : caller_id; - - this->owner_context = context; - - pthread_mutex_unlock(&this->guard); +void CyLock::rlock(CYTHON_UNUSED const char *context) { + pthread_rwlock_rdlock(&this->rwlock); } int CyLock::tryrlock() { - pid_t caller_id = syscall(SYS_gettid); - - if (this->owner_id == caller_id) { - ++this->readers_nb; - return 0; - } - - // we must lock here, because a trylock could fail also when another thread is currently read-locking or read-unlocking - // but this means we might miss a writer arriving and leaving - pthread_mutex_lock(&this->guard); - - if (this->write_count > 0) { - pthread_mutex_unlock(&this->guard); - return CyObject_CONTENDING_WRITER_FLAG; - } - - this->owner_id = this->readers_nb++ ? CyObject_MANY_OWNERS : caller_id; - - pthread_mutex_unlock(&this->guard); - - return 0; + return pthread_rwlock_tryrdlock(&this->rwlock); } void CyLock::unrlock() { - pthread_mutex_lock(&this->guard); - - if (--this->readers_nb == 0) { - if (this->write_count == 0) { - this->owner_id = CyObject_NO_OWNER; - } - - // broadcast to wake up all the waiting writers - pthread_cond_broadcast(&this->readers_have_left); - } - - pthread_mutex_unlock(&this->guard); + pthread_rwlock_unlock(&this->rwlock); } -void CyLock::wlock(const char *context) { - pid_t caller_id = syscall(SYS_gettid); - - if (this->owner_id == caller_id) { - if (this->write_count) { - ++this->write_count; - return; - } - } - - pthread_mutex_lock(&this->guard); - - pid_t owner_id = this->owner_id; - - if (owner_id != caller_id) { - - // Since we use a reader-preferring approach, we wait first for all readers to leave, and then all writers. - // The other way around could result in several writers acquiring the lock. - - if (this->readers_nb > 0) { - #if CyObject_RAISE_ON_CONTENTION - pid_t owner_id = this->owner_id; - std::ostringstream msg; - msg << "Data Race between [this] writer #" << caller_id - << " and [other] reader #" << owner_id - << " on lock " << this; - if (context != NULL) { - msg << std::endl << "In [this] context: " << context; - } - if (this->owner_context != NULL) { - msg << std::endl << "In [other] context: " << this->owner_context; - } - throw std::runtime_error(msg.str()); - #else - pthread_mutex_lock(&(CyLock::log_guard)); - std::cerr - << "Data Race between [this] writer #" << caller_id - << " and [other] reader #" << owner_id - << " on lock " << this << std::endl; - if (context != NULL) { - std::cerr << "In [this] context: " << context << std::endl; - } - if (this->owner_context != NULL) { - std::cerr << "In [other] context: " << this->owner_context << std::endl; - } - pthread_mutex_unlock(&(CyLock::log_guard)); - #endif - } - - while (this->readers_nb > 0) { - pthread_cond_wait(&this->readers_have_left, &this->guard); - } - - if (this->write_count > 0) { - #if CyObject_RAISE_ON_CONTENTION - pid_t owner_id = this->owner_id; - std::ostringstream msg; - msg << "Data Race between [this] writer #" << caller_id - << " and [other] writer #" << owner_id - << " on lock " << this; - if (context != NULL) { - msg << std::endl << "In [this] context: " << context; - } - if (this->owner_context != NULL) { - msg << std::endl << "In [other] context: " << this->owner_context; - } - throw std::runtime_error(msg.str()); - #else - pthread_mutex_lock(&(CyLock::log_guard)); - std::cerr - << "Data Race between [this] writer #" << caller_id - << " and [other] writer #" << owner_id - << " on lock " << this << std::endl; - if (context != NULL) { - std::cerr << "In [this] context: " << context << std::endl; - } - if (this->owner_context != NULL) { - std::cerr << "In [other] context: " << this->owner_context << std::endl; - } - pthread_mutex_unlock(&(CyLock::log_guard)); - #endif - } - - while (this->write_count > 0) { - pthread_cond_wait(&this->writer_has_left, &this->guard); - } - - this->owner_id = caller_id; - } - - this->write_count = 1; - - this->owner_context = context; - - pthread_mutex_unlock(&this->guard); +void CyLock::wlock(CYTHON_UNUSED const char *context) { + pthread_rwlock_wrlock(&this->rwlock); } int CyLock::trywlock() { - pid_t caller_id = syscall(SYS_gettid); - - if (this->owner_id == caller_id) { - if (this->write_count) { - ++this->write_count; - return 0; - } - } - - pthread_mutex_lock(&this->guard); - - if (this->owner_id != caller_id) { - - if (this->readers_nb > 0) { - pthread_mutex_unlock(&this->guard); - return CyObject_CONTENDING_READER_FLAG; - } - - if (this->write_count > 0) { - pthread_mutex_unlock(&this->guard); - return CyObject_CONTENDING_WRITER_FLAG; - } - - this->owner_id = caller_id; - } - - this->write_count = 1; - - pthread_mutex_unlock(&this->guard); - - return 0; + return pthread_rwlock_trywrlock(&this->rwlock); } void CyLock::unwlock() { - pthread_mutex_lock(&this->guard); - if (--this->write_count == 0) { - if (this->readers_nb == 0) { - this->owner_id = CyObject_NO_OWNER; - } - - // broadcast to wake up all the waiting readers, + maybe one waiting writer - // more efficient to count r waiting readers and w waiting writers and signal n + (w > 0) times - pthread_cond_broadcast(&this->writer_has_left); - } - pthread_mutex_unlock(&this->guard); + pthread_rwlock_unlock(&this->rwlock); } -- 2.30.9