Don't generate a mutex for each Locked<T>, share one per object

This reduces RAM usage per object by sizeof(mutex)*(FIELDS-1).

lib/base/atomic.hpp

@@ -5,8 +5,6 @@
 
 #include <atomic>
 #include <mutex>
-#include <type_traits>
-#include <utility>
 
 namespace icinga
 {
@@ -34,8 +32,10 @@ public:
 	}
 };
 
+class LockedMutex;
+
 /**
- * Wraps any T into a std::atomic<T>-like interface that locks using a mutex.
+ * Wraps any T into an interface similar to std::atomic<T>, that locks using a mutex.
  *
  * In contrast to std::atomic<T>, Locked<T> is also valid for types that are not trivially copyable.
  * In case T is trivially copyable, std::atomic<T> is almost certainly the better choice.
@@ -46,38 +46,44 @@ template<typename T>
 class Locked
 {
 public:
-	inline T load() const
+	T load(LockedMutex& mtx) const;
-	{
+	void store(T desired, LockedMutex& mtx);
-		std::unique_lock<std::mutex> lock(m_Mutex);
-
-		return m_Value;
-	}
-
-	inline void store(T desired)
-	{
-		std::unique_lock<std::mutex> lock(m_Mutex);
-
-		m_Value = std::move(desired);
-	}
 
 private:
-	mutable std::mutex m_Mutex;
 	T m_Value;
 };
 
 /**
- * Type alias for std::atomic<T> if possible, otherwise Locked<T> is used as a fallback.
+ * Wraps std::mutex, so that only Locked<T> can (un)lock it.
+ *
+ * The latter tiny lock scope is enforced this way to prevent deadlocks while passing around mutexes.
  *
  * @ingroup base
  */
-template <typename T>
+class LockedMutex
-using AtomicOrLocked =
+{
-#if defined(__GNUC__) && __GNUC__ < 5
+	template<class T>
-	// GCC does not implement std::is_trivially_copyable until version 5.
+	friend class Locked;
-	typename std::conditional<std::is_fundamental<T>::value || std::is_pointer<T>::value, std::atomic<T>, Locked<T>>::type;
+
-#else /* defined(__GNUC__) && __GNUC__ < 5 */
+private:
-	typename std::conditional<std::is_trivially_copyable<T>::value, std::atomic<T>, Locked<T>>::type;
+	std::mutex m_Mutex;
-#endif /* defined(__GNUC__) && __GNUC__ < 5 */
+};
+
+template<class T>
+T Locked<T>::load(LockedMutex& mtx) const
+{
+	std::unique_lock lock (mtx.m_Mutex);
+
+	return m_Value;
+}
+
+template<class T>
+void Locked<T>::store(T desired, LockedMutex& mtx)
+{
+	std::unique_lock lock (mtx.m_Mutex);
+
+	m_Value = std::move(desired);
+}
 
 }
 

lib/base/configobject.ti

@@ -59,7 +59,7 @@ abstract class ConfigObject : ConfigObjectBase < ConfigType
 	[config, no_user_modify] String __name (Name);
 	[config, no_user_modify, required] String "name" (ShortName) {
 		get {{{
-			String shortName = m_ShortName.load();
+			String shortName = m_ShortName.load(m_FieldsMutex);
 			if (shortName.IsEmpty())
 				return GetName();
 			else

lib/icinga/host.ti

@@ -21,7 +21,7 @@ class Host : Checkable
 
 	[config] String display_name {
 		get {{{
-			String displayName = m_DisplayName.load();
+			String displayName = m_DisplayName.load(m_FieldsMutex);
 			if (displayName.IsEmpty())
 				return GetName();
 			else

lib/icinga/hostgroup.ti

@@ -11,7 +11,7 @@ class HostGroup : CustomVarObject
 {
 	[config] String display_name {
 		get {{{
-			String displayName = m_DisplayName.load();
+			String displayName = m_DisplayName.load(m_FieldsMutex);
 			if (displayName.IsEmpty())
 				return GetName();
 			else

lib/icinga/service.ti

@@ -33,7 +33,7 @@ class Service : Checkable < ServiceNameComposer
 
 	[config] String display_name {
 		get {{{
-			String displayName = m_DisplayName.load();
+			String displayName = m_DisplayName.load(m_FieldsMutex);
 			if (displayName.IsEmpty())
 				return GetShortName();
 			else

lib/icinga/servicegroup.ti

@@ -11,7 +11,7 @@ class ServiceGroup : CustomVarObject
 {
 	[config] String display_name {
 		get {{{
-			String displayName = m_DisplayName.load();
+			String displayName = m_DisplayName.load(m_FieldsMutex);
 			if (displayName.IsEmpty())
 				return GetName();
 			else

lib/icinga/timeperiod.ti

@@ -12,7 +12,7 @@ class TimePeriod : CustomVarObject
 {
 	[config] String display_name {
 		get {{{
-			String displayName = m_DisplayName.load();
+			String displayName = m_DisplayName.load(m_FieldsMutex);
 			if (displayName.IsEmpty())
 				return GetName();
 			else

lib/icinga/user.ti

@@ -13,7 +13,7 @@ class User : CustomVarObject
 {
 	[config] String display_name {
 		get {{{
-			String displayName = m_DisplayName.load();
+			String displayName = m_DisplayName.load(m_FieldsMutex);
 			if (displayName.IsEmpty())
 				return GetName();
 			else

lib/icinga/usergroup.ti

@@ -11,7 +11,7 @@ class UserGroup : CustomVarObject
 {
 	[config] String display_name {
 		get {{{
-			String displayName = m_DisplayName.load();
+			String displayName = m_DisplayName.load(m_FieldsMutex);
 			if (displayName.IsEmpty())
 				return GetName();
 			else

lib/icingadb/icingadb.cpp

@@ -32,7 +32,7 @@ REGISTER_TYPE(IcingaDB);
 IcingaDB::IcingaDB()
 	: m_Rcon(nullptr)
 {
-	m_RconLocked.store(nullptr);
+	m_RconLocked.store(nullptr, m_FieldsMutex);
 
 	m_WorkQueue.SetName("IcingaDB");
 
@@ -84,7 +84,7 @@ void IcingaDB::Start(bool runtimeCreated)
 	m_Rcon = new RedisConnection(GetHost(), GetPort(), GetPath(), GetUsername(), GetPassword(), GetDbIndex(),
 		GetEnableTls(), GetInsecureNoverify(), GetCertPath(), GetKeyPath(), GetCaPath(), GetCrlPath(),
 		GetTlsProtocolmin(), GetCipherList(), GetConnectTimeout(), GetDebugInfo());
-	m_RconLocked.store(m_Rcon);
+	m_RconLocked.store(m_Rcon, m_FieldsMutex);
 
 	for (const Type::Ptr& type : GetTypes()) {
 		auto ctype (dynamic_cast<ConfigType*>(type.get()));

lib/icingadb/icingadb.hpp

@@ -48,7 +48,7 @@ public:
 
 	inline RedisConnection::Ptr GetConnection()
 	{
-		return m_RconLocked.load();
+		return m_RconLocked.load(m_FieldsMutex);
 	}
 
 	template<class T>

tools/mkclass/classcompiler.cpp

@@ -454,8 +454,14 @@ void ClassCompiler::HandleClass(const Klass& klass, const ClassDebugInfo&)
 	m_Header << "template<>" << std::endl
 		<< "class ObjectImpl<" << klass.Name << ">"
 		<< " : public " << (klass.Parent.empty() ? "Object" : klass.Parent) << std::endl
-		<< "{" << std::endl
+		<< "{" << std::endl;
-		<< "public:" << std::endl
+
+	if (klass.Parent.empty()) {
+		m_Header << "protected:" << std::endl
+			<< "\tmutable LockedMutex m_FieldsMutex;" << std::endl << std::endl;
+	}
+
+	m_Header << "public:" << std::endl
 		<< "\t" << "DECLARE_PTR_TYPEDEFS(ObjectImpl<" << klass.Name << ">);" << std::endl << std::endl;
 
 	/* Validate */
@@ -815,7 +821,7 @@ void ClassCompiler::HandleClass(const Klass& klass, const ClassDebugInfo&)
 					<< "{" << std::endl;
 
 				if (field.GetAccessor.empty() && !(field.Attributes & FANoStorage))
-					m_Impl << "\t" << "return m_" << field.GetFriendlyName() << ".load();" << std::endl;
+					m_Impl << "\treturn m_" << field.GetFriendlyName() << ".load(m_FieldsMutex);" << std::endl;
 				else
 					m_Impl << field.GetAccessor << std::endl;
 
@@ -853,7 +859,7 @@ void ClassCompiler::HandleClass(const Klass& klass, const ClassDebugInfo&)
 						<< "\t" << "auto *dobj = dynamic_cast<ConfigObject *>(this);" << std::endl;
 
 				if (field.SetAccessor.empty() && !(field.Attributes & FANoStorage))
-					m_Impl << "\t" << "m_" << field.GetFriendlyName() << ".store(value);" << std::endl;
+					m_Impl << "\tm_" << field.GetFriendlyName() << ".store(value, m_FieldsMutex);" << std::endl;
 				else
 					m_Impl << field.SetAccessor << std::endl << std::endl;
 
@@ -1068,7 +1074,7 @@ void ClassCompiler::HandleClass(const Klass& klass, const ClassDebugInfo&)
 			if (field.Attributes & FANoStorage)
 				continue;
 
-			m_Header << "\tAtomicOrLocked<" << field.Type.GetRealType() << "> m_" << field.GetFriendlyName() << ";" << std::endl;
+			m_Header << "\tLocked<" << field.Type.GetRealType() << "> m_" << field.GetFriendlyName() << ";" << std::endl;
 		}
 		
 		/* signal */