Merge pull request #9364 from Icinga/bugfix/atomic-members

Replace `EventuallyAtomic` with `AtomicOrLocked` which falls back to a mutex

lib/base/atomic.hpp

@@ -4,6 +4,7 @@
 #define ATOMIC_H
 
 #include <atomic>
+#include <mutex>
 #include <type_traits>
 #include <utility>
 
@@ -41,78 +42,49 @@ public:
 };
 
 /**
- * Wraps T into a std::atomic<T>-like interface.
+ * Wraps any T into a std::atomic<T>-like interface 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.
  *
  * @ingroup base
  */
-template<class T>
+template<typename T>
-class NotAtomic
+class Locked
 {
 public:
 	inline T load() const
 	{
+		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;
 };
 
 /**
- * Tells whether to use std::atomic<T> or NotAtomic<T>.
+ * Type alias for std::atomic<T> if possible, otherwise Locked<T> is used as a fallback.
  *
  * @ingroup base
  */
-template<class T>
+template <typename T>
-struct Atomicable
+using AtomicOrLocked =
-{
+#if defined(__GNUC__) && __GNUC__ < 5
-	// Doesn't work with too old compilers.
+	// GCC does not implement std::is_trivially_copyable until version 5.
-	//static constexpr bool value = std::is_trivially_copyable<T>::value && sizeof(T) <= sizeof(void*);
+	typename std::conditional<std::is_fundamental<T>::value || std::is_pointer<T>::value, std::atomic<T>, Locked<T>>::type;
-	static constexpr bool value = (std::is_fundamental<T>::value || std::is_pointer<T>::value) && sizeof(T) <= sizeof(void*);
+#else /* defined(__GNUC__) && __GNUC__ < 5 */
-};
+	typename std::conditional<std::is_trivially_copyable<T>::value, std::atomic<T>, Locked<T>>::type;
-
+#endif /* defined(__GNUC__) && __GNUC__ < 5 */
-/**
- * Uses either std::atomic<T> or NotAtomic<T> depending on atomicable.
- *
- * @ingroup base
- */
-template<bool atomicable>
-struct AtomicTemplate;
-
-template<>
-struct AtomicTemplate<false>
-{
-	template<class T>
-	struct tmplt
-	{
-		typedef NotAtomic<T> type;
-	};
-};
-
-template<>
-struct AtomicTemplate<true>
-{
-	template<class T>
-	struct tmplt
-	{
-		typedef std::atomic<T> type;
-	};
-};
-
-/**
- * Uses either std::atomic<T> or NotAtomic<T> depending on T.
- *
- * @ingroup base
- */
-template<class T>
-struct EventuallyAtomic
-{
-	typedef typename AtomicTemplate<Atomicable<T>::value>::template tmplt<T>::type type;
-};
 
 }
 

lib/base/configobject.ti

@@ -59,10 +59,11 @@ abstract class ConfigObject : ConfigObjectBase < ConfigType
 	[config, no_user_modify] String __name (Name);
 	[config, no_user_modify] String "name" (ShortName) {
 		get {{{
-			if (m_ShortName.m_Value.IsEmpty())
+			String shortName = m_ShortName.load();
+			if (shortName.IsEmpty())
 				return GetName();
 			else
-				return m_ShortName.m_Value;
+				return shortName;
 		}}}
 	};
 	[config, no_user_modify] name(Zone) zone (ZoneName);

lib/icinga/host.ti

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

lib/icinga/hostgroup.ti

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

lib/icinga/service.ti

@@ -33,10 +33,11 @@ class Service : Checkable < ServiceNameComposer
 
 	[config] String display_name {
 		get {{{
-			if (m_DisplayName.m_Value.IsEmpty())
+			String displayName = m_DisplayName.load();
+			if (displayName.IsEmpty())
 				return GetShortName();
 			else
-				return m_DisplayName.m_Value;
+				return displayName;
 		}}}
 	};
 	[config, required] name(Host) host_name;

lib/icinga/servicegroup.ti

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

lib/icinga/timeperiod.ti

@@ -12,10 +12,11 @@ class TimePeriod : CustomVarObject
 {
 	[config] String display_name {
 		get {{{
-			if (m_DisplayName.m_Value.IsEmpty())
+			String displayName = m_DisplayName.load();
+			if (displayName.IsEmpty())
 				return GetName();
 			else
-				return m_DisplayName.m_Value;
+				return displayName;
 		}}}
 	};
 	[config, signal_with_old_value] Dictionary::Ptr ranges;

lib/icinga/user.ti

@@ -13,10 +13,11 @@ class User : CustomVarObject
 {
 	[config] String display_name {
 		get {{{
-			if (m_DisplayName.m_Value.IsEmpty())
+			String displayName = m_DisplayName.load();
+			if (displayName.IsEmpty())
 				return GetName();
 			else
-				return m_DisplayName.m_Value;
+				return displayName;
 		}}}
 	};
 	[config, no_user_modify, required, signal_with_old_value] array(name(UserGroup)) groups {

lib/icinga/usergroup.ti

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

tools/mkclass/classcompiler.cpp

@@ -1060,7 +1060,7 @@ void ClassCompiler::HandleClass(const Klass& klass, const ClassDebugInfo&)
 			if (field.Attributes & FANoStorage)
 				continue;
 
-			m_Header << "\tEventuallyAtomic<" << field.Type.GetRealType() << ">::type m_" << field.GetFriendlyName() << ";" << std::endl;
+			m_Header << "\tAtomicOrLocked<" << field.Type.GetRealType() << "> m_" << field.GetFriendlyName() << ";" << std::endl;
 		}
 		
 		/* signal */