Merge pull request #7128 from Icinga/feature/re-write-objectlock-7123

Re-write ObjectLock's implementation details

lib/base/object.cpp

@@ -10,6 +10,7 @@
 #include "base/exception.hpp"
 #include <boost/lexical_cast.hpp>
 #include <boost/thread/recursive_mutex.hpp>
+#include <thread>
 
 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())());
 }
 
 /**
@@ -34,7 +36,6 @@ Object::Object()
  */
 Object::~Object()
 {
-	delete reinterpret_cast<boost::recursive_mutex *>(m_Mutex);
 }
 
 /**
@@ -53,15 +54,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 */
 

lib/base/object.hpp

@@ -9,6 +9,8 @@
 #include <atomic>
 #include <cstddef>
 #include <cstdint>
+#include <mutex>
+#include <thread>
 #include <vector>
 
 using boost::intrusive_ptr;
@@ -190,14 +192,10 @@ private:
 	Object& operator=(const Object& rhs) = delete;
 
 	std::atomic<uint_fast64_t> m_References;
-	mutable uintptr_t m_Mutex{0};
+	mutable std::recursive_mutex m_Mutex;
 
 #ifdef I2_DEBUG
-#	ifndef _WIN32
-	mutable pthread_t m_LockOwner;
-#	else /* _WIN32 */
-	mutable DWORD m_LockOwner;
-#	endif /* _WIN32 */
+	mutable std::atomic<std::thread::id> m_LockOwner;
 	mutable size_t m_LockCount = 0;
 #endif /* I2_DEBUG */
 

lib/base/objectlock.cpp

@@ -1,7 +1,7 @@
 /* Icinga 2 | (c) 2012 Icinga GmbH | GPLv2+ */
 
 #include "base/objectlock.hpp"
-#include <boost/thread/recursive_mutex.hpp>
+#include <thread>
 
 using namespace icinga;
 
@@ -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)
@@ -27,95 +25,31 @@ 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<boost::recursive_mutex *>(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<LONGLONG>(mtx), I2MUTEX_LOCKED);
-#	else /* _WIN64 */
-	InterlockedCompareExchange(&object->m_Mutex, reinterpret_cast<LONG>(mtx), I2MUTEX_LOCKED);
-#	endif /* _WIN64 */
-#else /* _WIN32 */
-	__sync_bool_compare_and_swap(&object->m_Mutex, I2MUTEX_LOCKED, reinterpret_cast<uintptr_t>(mtx));
-#endif /* _WIN32 */
-}
-
 void ObjectLock::Lock()
 {
 	ASSERT(!m_Locked && m_Object);
 
-	LockMutex(m_Object);
+	m_Object->m_Mutex.lock();
 
 	m_Locked = true;
 
 #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 */
 }
 
-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
 	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 */
 
 	if (m_Locked) {
-		reinterpret_cast<boost::recursive_mutex *>(m_Object->m_Mutex)->unlock();
+		m_Object->m_Mutex.unlock();
 		m_Locked = false;
 	}
 }

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: