Merge pull request Icinga#9627 from Icinga/namespace-shared-mutex

Namespace: use rwlock and disable read locking after freeze

icinga-app/icinga.cpp

@@ -280,17 +280,6 @@ static int Main()
 #endif /* RLIMIT_STACK */
 	}
 
-	/* Calculate additional global constants. */
-	ScriptGlobal::Set("System.PlatformKernel", Utility::GetPlatformKernel(), true);
-	ScriptGlobal::Set("System.PlatformKernelVersion", Utility::GetPlatformKernelVersion(), true);
-	ScriptGlobal::Set("System.PlatformName", Utility::GetPlatformName(), true);
-	ScriptGlobal::Set("System.PlatformVersion", Utility::GetPlatformVersion(), true);
-	ScriptGlobal::Set("System.PlatformArchitecture", Utility::GetPlatformArchitecture(), true);
-
-	ScriptGlobal::Set("System.BuildHostName", ICINGA_BUILD_HOST_NAME, true);
-	ScriptGlobal::Set("System.BuildCompilerName", ICINGA_BUILD_COMPILER_NAME, true);
-	ScriptGlobal::Set("System.BuildCompilerVersion", ICINGA_BUILD_COMPILER_VERSION, true);
-
 	if (!autocomplete)
 		Application::SetResourceLimits();
 

lib/base/function.hpp

@@ -60,28 +60,28 @@ class Function final : public ObjectImpl<Function>
 	INITIALIZE_ONCE_WITH_PRIORITY([]() { \
 		Function::Ptr sf = new icinga::Function(#ns "#" #name, callback, String(args).Split(":"), false); \
 		Namespace::Ptr nsp = ScriptGlobal::Get(#ns); \
-		nsp->SetAttribute(#name, new ConstEmbeddedNamespaceValue(sf)); \
+		nsp->Set(#name, sf, true); \
 	}, InitializePriority::RegisterFunctions)
 
 #define REGISTER_SAFE_FUNCTION(ns, name, callback, args) \
 	INITIALIZE_ONCE_WITH_PRIORITY([]() { \
 		Function::Ptr sf = new icinga::Function(#ns "#" #name, callback, String(args).Split(":"), true); \
 		Namespace::Ptr nsp = ScriptGlobal::Get(#ns); \
-		nsp->SetAttribute(#name, new ConstEmbeddedNamespaceValue(sf)); \
+		nsp->Set(#name, sf, true); \
 	}, InitializePriority::RegisterFunctions)
 
 #define REGISTER_FUNCTION_NONCONST(ns, name, callback, args) \
 	INITIALIZE_ONCE_WITH_PRIORITY([]() { \
 		Function::Ptr sf = new icinga::Function(#ns "#" #name, callback, String(args).Split(":"), false); \
 		Namespace::Ptr nsp = ScriptGlobal::Get(#ns); \
-		nsp->SetAttribute(#name, new EmbeddedNamespaceValue(sf)); \
+		nsp->Set(#name, sf, false); \
 	}, InitializePriority::RegisterFunctions)
 
 #define REGISTER_SAFE_FUNCTION_NONCONST(ns, name, callback, args) \
 	INITIALIZE_ONCE_WITH_PRIORITY([]() { \
 		Function::Ptr sf = new icinga::Function(#ns "#" #name, callback, String(args).Split(":"), true); \
 		Namespace::Ptr nsp = ScriptGlobal::Get(#ns); \
-		nsp->SetAttribute(#name, new EmbeddedNamespaceValue(sf)); \
+		nsp->SetAttribute(#name, sf, false); \
 	}, InitializePriority::RegisterFunctions)
 
 }

lib/base/initialize.hpp

@@ -25,6 +25,7 @@ enum class InitializePriority {
 	RegisterTypes,
 	EvaluateConfigFragments,
 	Default,
+	FreezeNamespaces,
 };
 
 #define I2_TOKENPASTE(x, y) x ## y

lib/base/json-script.cpp

@@ -24,5 +24,5 @@ INITIALIZE_ONCE([]() {
 	jsonNS->Freeze();
 
 	Namespace::Ptr systemNS = ScriptGlobal::Get("System");
-	systemNS->SetAttribute("Json", new ConstEmbeddedNamespaceValue(jsonNS));
+	systemNS->Set("Json", jsonNS, true);
 });

lib/base/json.cpp

@@ -95,7 +95,7 @@ void EncodeNamespace(JsonEncoder<prettyPrint>& stateMachine, const Namespace::Pt
 	ObjectLock olock(ns);
 	for (const Namespace::Pair& kv : ns) {
 		stateMachine.Key(Utility::ValidateUTF8(kv.first));
-		Encode(stateMachine, kv.second->Get());
+		Encode(stateMachine, kv.second.Val);
 	}
 
 	stateMachine.EndObject();

lib/base/logger.cpp

@@ -40,11 +40,11 @@ std::mutex Logger::m_UpdateMinLogSeverityMutex;
 Atomic<LogSeverity> Logger::m_MinLogSeverity (LogDebug);
 
 INITIALIZE_ONCE([]() {
-	ScriptGlobal::Set("System.LogDebug", LogDebug, true);
-	ScriptGlobal::Set("System.LogNotice", LogNotice, true);
-	ScriptGlobal::Set("System.LogInformation", LogInformation, true);
-	ScriptGlobal::Set("System.LogWarning", LogWarning, true);
-	ScriptGlobal::Set("System.LogCritical", LogCritical, true);
+	ScriptGlobal::Set("System.LogDebug", LogDebug);
+	ScriptGlobal::Set("System.LogNotice", LogNotice);
+	ScriptGlobal::Set("System.LogInformation", LogInformation);
+	ScriptGlobal::Set("System.LogWarning", LogWarning);
+	ScriptGlobal::Set("System.LogCritical", LogCritical);
 });
 
 /**

lib/base/math-script.cpp

@@ -180,5 +180,5 @@ INITIALIZE_ONCE([]() {
 	mathNS->Freeze();
 
 	Namespace::Ptr systemNS = ScriptGlobal::Get("System");
-	systemNS->SetAttribute("Math", new ConstEmbeddedNamespaceValue(mathNS));
+	systemNS->Set("Math", mathNS, true);
 });

lib/base/namespace-script.cpp

@@ -63,7 +63,7 @@ static Array::Ptr NamespaceValues()
 	ArrayData values;
 	ObjectLock olock(self);
 	for (const Namespace::Pair& kv : self) {
-		values.push_back(kv.second->Get());
+		values.push_back(kv.second.Val);
 	}
 	return new Array(std::move(values));
 }

lib/base/namespace.cpp

@@ -25,32 +25,47 @@ Namespace::Namespace(bool constValues)
 
 Value Namespace::Get(const String& field) const
 {
-	ObjectLock olock(this);
-
 	Value value;
-	if (!GetOwnField(field, &value))
+	if (!Get(field, &value))
 		BOOST_THROW_EXCEPTION(ScriptError("Namespace does not contain field '" + field + "'"));
 	return value;
 }
 
 bool Namespace::Get(const String& field, Value *value) const
 {
-	ObjectLock olock(this);
+	auto lock(ReadLockUnlessFrozen());
 
-	auto nsVal = GetAttribute(field);
+	auto nsVal = m_Data.find(field);
 
-	if (!nsVal)
+	if (nsVal == m_Data.end()) {
 		return false;
+	}
 
-	*value =  nsVal->Get(DebugInfo());
+	*value = nsVal->second.Val;
 	return true;
 }
 
-void Namespace::Set(const String& field, const Value& value, bool overrideFrozen)
+void Namespace::Set(const String& field, const Value& value, bool isConst, const DebugInfo& debugInfo)
 {
 	ObjectLock olock(this);
 
-	return SetFieldByName(field, value, overrideFrozen, DebugInfo());
+	if (m_Frozen) {
+		BOOST_THROW_EXCEPTION(ScriptError("Namespace is read-only and must not be modified.", debugInfo));
+	}
+
+	std::unique_lock<decltype(m_DataMutex)> dlock (m_DataMutex);
+
+	auto nsVal = m_Data.find(field);
+
+	if (nsVal == m_Data.end()) {
+		m_Data[field] = NamespaceValue{value, isConst};
+	} else {
+		if (nsVal->second.Const) {
+			BOOST_THROW_EXCEPTION(ScriptError("Constant must not be modified.", debugInfo));
+		}
+
+		nsVal->second.Val = value;
+	}
 }
 
 /**
@@ -60,39 +75,43 @@ void Namespace::Set(const String& field, const Value& value, bool overrideFrozen
  */
 size_t Namespace::GetLength() const
 {
-	ObjectLock olock(this);
+	auto lock(ReadLockUnlessFrozen());
 
 	return m_Data.size();
 }
 
 bool Namespace::Contains(const String& field) const
 {
-	ObjectLock olock(this);
+	auto lock (ReadLockUnlessFrozen());
 
-	return HasOwnField(field);
+	return m_Data.find(field) != m_Data.end();
 }
 
-void Namespace::Remove(const String& field, bool overrideFrozen)
+void Namespace::Remove(const String& field)
 {
 	ObjectLock olock(this);
 
-	if (m_Frozen && !overrideFrozen) {
+	if (m_Frozen) {
 		BOOST_THROW_EXCEPTION(ScriptError("Namespace is read-only and must not be modified."));
 	}
 
-	if (!overrideFrozen) {
-		auto attr = GetAttribute(field);
+	std::unique_lock<decltype(m_DataMutex)> dlock (m_DataMutex);
 
-		if (dynamic_pointer_cast<ConstEmbeddedNamespaceValue>(attr)) {
-			BOOST_THROW_EXCEPTION(ScriptError("Constants must not be removed."));
-		}
+	auto it = m_Data.find(field);
+
+	if (it == m_Data.end()) {
+		return;
 	}
 
-	RemoveAttribute(field);
+	if (it->second.Const) {
+		BOOST_THROW_EXCEPTION(ScriptError("Constants must not be removed."));
+	}
+
+	m_Data.erase(it);
 }
 
 /**
- * Freeze the namespace, preventing further updates unless overrideFrozen is set.
+ * Freeze the namespace, preventing further updates.
  *
  * This only prevents inserting, replacing or deleting values from the namespace. This operation has no effect on
  * objects referenced by the values, these remain mutable if they were before.
@@ -103,111 +122,45 @@ void Namespace::Freeze() {
 	m_Frozen = true;
 }
 
-void Namespace::RemoveAttribute(const String& field)
-{
-	ObjectLock olock(this);
-
-	Namespace::Iterator it;
-	it = m_Data.find(field);
-
-	if (it == m_Data.end())
-		return;
-
-	m_Data.erase(it);
-}
-
-NamespaceValue::Ptr Namespace::GetAttribute(const String& key) const
-{
-	ObjectLock olock(this);
-
-	auto it = m_Data.find(key);
-
-	if (it == m_Data.end())
-		return nullptr;
-
-	return it->second;
-}
-
-void Namespace::SetAttribute(const String& key, const NamespaceValue::Ptr& nsVal)
+std::shared_lock<std::shared_timed_mutex> Namespace::ReadLockUnlessFrozen() const
 {
-	ObjectLock olock(this);
-
-	m_Data[key] = nsVal;
+	if (m_Frozen.load(std::memory_order_relaxed)) {
+		return std::shared_lock<std::shared_timed_mutex>();
+	} else {
+		return std::shared_lock<std::shared_timed_mutex>(m_DataMutex);
+	}
 }
 
 Value Namespace::GetFieldByName(const String& field, bool, const DebugInfo& debugInfo) const
 {
-	ObjectLock olock(this);
+	auto lock (ReadLockUnlessFrozen());
 
-	auto nsVal = GetAttribute(field);
+	auto nsVal = m_Data.find(field);
 
-	if (nsVal)
-		return nsVal->Get(debugInfo);
+	if (nsVal != m_Data.end())
+		return nsVal->second.Val;
 	else
 		return GetPrototypeField(const_cast<Namespace *>(this), field, false, debugInfo); /* Ignore indexer not found errors similar to the Dictionary class. */
 }
 
 void Namespace::SetFieldByName(const String& field, const Value& value, bool overrideFrozen, const DebugInfo& debugInfo)
 {
-	ObjectLock olock(this);
-
-	auto nsVal = GetAttribute(field);
+	// The override frozen parameter is mandated by the interface but ignored here. If the namespace is frozen, this
+	// disables locking for read operations, so it must not be modified again to ensure the consistency of the internal
+	// data structures.
+	(void) overrideFrozen;
 
-	if (!nsVal) {
-		if (m_Frozen && !overrideFrozen) {
-			BOOST_THROW_EXCEPTION(ScriptError("Namespace is read-only and must not be modified.", debugInfo));
-		}
-
-		if (m_ConstValues) {
-			SetAttribute(field, new ConstEmbeddedNamespaceValue(value));
-		} else {
-			SetAttribute(field, new EmbeddedNamespaceValue(value));
-		}
-	} else {
-		nsVal->Set(value, overrideFrozen, debugInfo);
-	}
+	Set(field, value, false, debugInfo);
 }
 
 bool Namespace::HasOwnField(const String& field) const
 {
-	ObjectLock olock(this);
-
-	return GetAttribute(field) != nullptr;
+	return Contains(field);
 }
 
 bool Namespace::GetOwnField(const String& field, Value *result) const
 {
-	ObjectLock olock(this);
-
-	auto nsVal = GetAttribute(field);
-
-	if (!nsVal)
-		return false;
-
-	*result = nsVal->Get(DebugInfo());
-	return true;
-}
-
-EmbeddedNamespaceValue::EmbeddedNamespaceValue(const Value& value)
-	: m_Value(value)
-{ }
-
-Value EmbeddedNamespaceValue::Get(const DebugInfo& debugInfo) const
-{
-	return m_Value;
-}
-
-void EmbeddedNamespaceValue::Set(const Value& value, bool, const DebugInfo&)
-{
-	m_Value = value;
-}
-
-void ConstEmbeddedNamespaceValue::Set(const Value& value, bool overrideFrozen, const DebugInfo& debugInfo)
-{
-	if (!overrideFrozen)
-		BOOST_THROW_EXCEPTION(ScriptError("Constant must not be modified.", debugInfo));
-
-	EmbeddedNamespaceValue::Set(value, overrideFrozen, debugInfo);
+	return Get(field, result);
 }
 
 Namespace::Iterator Namespace::Begin()