From f9f998334d6646cd85b903686eedca1e087a6c43 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Wed, 17 Apr 2019 16:47:41 +0200 Subject: [PATCH 1/3] ObjectLock: deduplicate constructors refs #7123 --- lib/base/objectlock.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/base/objectlock.cpp b/lib/base/objectlock.cpp index bc966cd80..c68710c9c 100644 --- a/lib/base/objectlock.cpp +++ b/lib/base/objectlock.cpp @@ -14,10 +14,8 @@ ObjectLock::~ObjectLock() } ObjectLock::ObjectLock(const Object::Ptr& object) - : m_Object(object.get()), m_Locked(false) + : ObjectLock(object.get()) { - if (m_Object) - Lock(); } ObjectLock::ObjectLock(const Object *object) From 7e6868bc99dab1a6da08a60351aaef38a243d2f1 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Wed, 17 Apr 2019 18:03:40 +0200 Subject: [PATCH 2/3] Make Object#m_LockOwner std::atomic refs #7123 --- lib/base/object.cpp | 12 +++--------- lib/base/object.hpp | 7 ++----- lib/base/objectlock.cpp | 13 +++---------- 3 files changed, 8 insertions(+), 24 deletions(-) diff --git a/lib/base/object.cpp b/lib/base/object.cpp index b00891b4c..1d40a753d 100644 --- a/lib/base/object.cpp +++ b/lib/base/object.cpp @@ -10,6 +10,7 @@ #include "base/exception.hpp" #include #include +#include using namespace icinga; @@ -27,6 +28,7 @@ static Timer::Ptr l_ObjectCountTimer; Object::Object() { m_References.store(0); + m_LockOwner.store(decltype(m_LockOwner.load())()); } /** @@ -53,15 +55,7 @@ String Object::ToString() const */ bool Object::OwnsLock() const { -#ifdef _WIN32 - DWORD tid = InterlockedExchangeAdd(&m_LockOwner, 0); - - return (tid == GetCurrentThreadId()); -#else /* _WIN32 */ - pthread_t tid = __sync_fetch_and_add(&m_LockOwner, 0); - - return (tid == pthread_self()); -#endif /* _WIN32 */ + return m_LockOwner.load() == std::this_thread::get_id(); } #endif /* I2_DEBUG */ diff --git a/lib/base/object.hpp b/lib/base/object.hpp index 71e9b0bfa..43cf7cee7 100644 --- a/lib/base/object.hpp +++ b/lib/base/object.hpp @@ -9,6 +9,7 @@ #include #include #include +#include #include using boost::intrusive_ptr; @@ -193,11 +194,7 @@ private: mutable uintptr_t m_Mutex{0}; #ifdef I2_DEBUG -# ifndef _WIN32 - mutable pthread_t m_LockOwner; -# else /* _WIN32 */ - mutable DWORD m_LockOwner; -# endif /* _WIN32 */ + mutable std::atomic m_LockOwner; mutable size_t m_LockCount = 0; #endif /* I2_DEBUG */ diff --git a/lib/base/objectlock.cpp b/lib/base/objectlock.cpp index c68710c9c..5a06488e9 100644 --- a/lib/base/objectlock.cpp +++ b/lib/base/objectlock.cpp @@ -2,6 +2,7 @@ #include "base/objectlock.hpp" #include +#include using namespace icinga; @@ -72,11 +73,7 @@ void ObjectLock::Lock() #ifdef I2_DEBUG if (++m_Object->m_LockCount == 1u) { -# ifdef _WIN32 - InterlockedExchange(&m_Object->m_LockOwner, GetCurrentThreadId()); -# else /* _WIN32 */ - __sync_lock_test_and_set(&m_Object->m_LockOwner, pthread_self()); -# endif /* _WIN32 */ + m_Object->m_LockOwner.store(std::this_thread::get_id()); } #endif /* I2_DEBUG */ } @@ -104,11 +101,7 @@ void ObjectLock::Unlock() { #ifdef I2_DEBUG if (m_Locked && !--m_Object->m_LockCount) { -# ifdef _WIN32 - InterlockedExchange(&m_Object->m_LockOwner, 0); -# else /* _WIN32 */ - __sync_lock_release(&m_Object->m_LockOwner); -# endif /* _WIN32 */ + m_Object->m_LockOwner.store(decltype(m_Object->m_LockOwner.load())()); } #endif /* I2_DEBUG */ From d8c9fdf1d4723896915b8e734d53dc78f70d1866 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Wed, 17 Apr 2019 18:15:32 +0200 Subject: [PATCH 3/3] Make Object#m_Mutex std::recursive_mutex refs #7123 --- lib/base/object.cpp | 1 - lib/base/object.hpp | 3 +- lib/base/objectlock.cpp | 61 ++--------------------------------------- lib/base/objectlock.hpp | 5 ---- 4 files changed, 4 insertions(+), 66 deletions(-) diff --git a/lib/base/object.cpp b/lib/base/object.cpp index 1d40a753d..58e2a9160 100644 --- a/lib/base/object.cpp +++ b/lib/base/object.cpp @@ -36,7 +36,6 @@ Object::Object() */ Object::~Object() { - delete reinterpret_cast(m_Mutex); } /** diff --git a/lib/base/object.hpp b/lib/base/object.hpp index 43cf7cee7..5a90cfa64 100644 --- a/lib/base/object.hpp +++ b/lib/base/object.hpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -191,7 +192,7 @@ private: Object& operator=(const Object& rhs) = delete; std::atomic m_References; - mutable uintptr_t m_Mutex{0}; + mutable std::recursive_mutex m_Mutex; #ifdef I2_DEBUG mutable std::atomic m_LockOwner; diff --git a/lib/base/objectlock.cpp b/lib/base/objectlock.cpp index 5a06488e9..fc0c7c631 100644 --- a/lib/base/objectlock.cpp +++ b/lib/base/objectlock.cpp @@ -1,7 +1,6 @@ /* Icinga 2 | (c) 2012 Icinga GmbH | GPLv2+ */ #include "base/objectlock.hpp" -#include #include using namespace icinga; @@ -26,48 +25,11 @@ ObjectLock::ObjectLock(const Object *object) Lock(); } -void ObjectLock::LockMutex(const Object *object) -{ - unsigned int it = 0; - -#ifdef _WIN32 -# ifdef _WIN64 - while (likely(InterlockedCompareExchange64((LONGLONG *)&object->m_Mutex, I2MUTEX_LOCKED, I2MUTEX_UNLOCKED) != I2MUTEX_UNLOCKED)) { -# else /* _WIN64 */ - while (likely(InterlockedCompareExchange(&object->m_Mutex, I2MUTEX_LOCKED, I2MUTEX_UNLOCKED) != I2MUTEX_UNLOCKED)) { -# endif /* _WIN64 */ -#else /* _WIN32 */ - while (likely(!__sync_bool_compare_and_swap(&object->m_Mutex, I2MUTEX_UNLOCKED, I2MUTEX_LOCKED))) { -#endif /* _WIN32 */ - if (likely(object->m_Mutex > I2MUTEX_LOCKED)) { - auto *mtx = reinterpret_cast(object->m_Mutex); - mtx->lock(); - - return; - } - - Spin(it); - it++; - } - - auto *mtx = new boost::recursive_mutex(); - mtx->lock(); -#ifdef _WIN32 -# ifdef _WIN64 - InterlockedCompareExchange64((LONGLONG *)&object->m_Mutex, reinterpret_cast(mtx), I2MUTEX_LOCKED); -# else /* _WIN64 */ - InterlockedCompareExchange(&object->m_Mutex, reinterpret_cast(mtx), I2MUTEX_LOCKED); -# endif /* _WIN64 */ -#else /* _WIN32 */ - __sync_bool_compare_and_swap(&object->m_Mutex, I2MUTEX_LOCKED, reinterpret_cast(mtx)); -#endif /* _WIN32 */ -} - void ObjectLock::Lock() { ASSERT(!m_Locked && m_Object); - LockMutex(m_Object); + m_Object->m_Mutex.lock(); m_Locked = true; @@ -78,25 +40,6 @@ void ObjectLock::Lock() #endif /* I2_DEBUG */ } -void ObjectLock::Spin(unsigned int it) -{ - if (it < 8) { - /* Do nothing. */ - } -#ifdef SPIN_PAUSE - else if (it < 16) { - SPIN_PAUSE(); - } -#endif /* SPIN_PAUSE */ - else { -#ifdef _WIN32 - Sleep(0); -#else /* _WIN32 */ - sched_yield(); -#endif /* _WIN32 */ - } -} - void ObjectLock::Unlock() { #ifdef I2_DEBUG @@ -106,7 +49,7 @@ void ObjectLock::Unlock() #endif /* I2_DEBUG */ if (m_Locked) { - reinterpret_cast(m_Object->m_Mutex)->unlock(); + m_Object->m_Mutex.unlock(); m_Locked = false; } } diff --git a/lib/base/objectlock.hpp b/lib/base/objectlock.hpp index 96cbac3d7..277f99041 100644 --- a/lib/base/objectlock.hpp +++ b/lib/base/objectlock.hpp @@ -19,12 +19,7 @@ public: ~ObjectLock(); - static void LockMutex(const Object *object); - void Lock(); - - static void Spin(unsigned int it); - void Unlock(); private: