Merge pull request #10523 from Icinga/dependency-eval-complexity

Prevent worst-case exponential complexity in dependency evaluation

doc/03-monitoring-basics.md

@@ -3097,6 +3097,12 @@ via the [REST API](12-icinga2-api.md#icinga2-api).
 > Reachability calculation depends on fresh and processed check results. If dependencies
 > disable checks for child objects, this won't work reliably.
 
+> **Note**
+>
+> The parent of a dependency can have a parent itself and so on. The nesting depth of
+> dependencies is currently limited to 256 which should be more than enough for any practical
+> use. This is an implementation detail and may change in the future.
+
 ### Implicit Dependencies for Services on Host <a id="dependencies-implicit-host-service"></a>
 
 Icinga 2 automatically adds an implicit dependency for services on their host. That way

lib/base/object.cpp

@@ -201,14 +201,14 @@ Value icinga::GetPrototypeField(const Value& context, const String& field, bool
 }
 
 #ifdef I2_LEAK_DEBUG
-void icinga::TypeAddObject(Object *object)
+void icinga::TypeAddObject(const Object *object)
 {
 	std::unique_lock<std::mutex> lock(l_ObjectCountLock);
 	String typeName = Utility::GetTypeName(typeid(*object));
 	l_ObjectCounts[typeName]++;
 }
 
-void icinga::TypeRemoveObject(Object *object)
+void icinga::TypeRemoveObject(const Object *object)
 {
 	std::unique_lock<std::mutex> lock(l_ObjectCountLock);
 	String typeName = Utility::GetTypeName(typeid(*object));
@@ -239,7 +239,7 @@ INITIALIZE_ONCE([]() {
 });
 #endif /* I2_LEAK_DEBUG */
 
-void icinga::intrusive_ptr_add_ref(Object *object)
+void icinga::intrusive_ptr_add_ref(const Object *object)
 {
 #ifdef I2_LEAK_DEBUG
 	if (object->m_References.fetch_add(1) == 0u)
@@ -249,7 +249,7 @@ void icinga::intrusive_ptr_add_ref(Object *object)
 #endif /* I2_LEAK_DEBUG */
 }
 
-void icinga::intrusive_ptr_release(Object *object)
+void icinga::intrusive_ptr_release(const Object *object)
 {
 	auto previous (object->m_References.fetch_sub(1));
 

lib/base/object.hpp

@@ -31,7 +31,8 @@ class ValidationUtils;
 extern const Value Empty;
 
 #define DECLARE_PTR_TYPEDEFS(klass) \
-	typedef intrusive_ptr<klass> Ptr
+	typedef intrusive_ptr<klass> Ptr; \
+	typedef intrusive_ptr<const klass> ConstPtr
 
 #define IMPL_TYPE_LOOKUP_SUPER() 					\
 
@@ -192,7 +193,7 @@ private:
 	Object(const Object& other) = delete;
 	Object& operator=(const Object& rhs) = delete;
 
-	std::atomic<uint_fast64_t> m_References;
+	mutable std::atomic<uint_fast64_t> m_References;
 	mutable std::recursive_mutex m_Mutex;
 
 #ifdef I2_DEBUG
@@ -202,17 +203,17 @@ private:
 
 	friend struct ObjectLock;
 
-	friend void intrusive_ptr_add_ref(Object *object);
+	friend void intrusive_ptr_add_ref(const Object *object);
-	friend void intrusive_ptr_release(Object *object);
+	friend void intrusive_ptr_release(const Object *object);
 };
 
 Value GetPrototypeField(const Value& context, const String& field, bool not_found_error, const DebugInfo& debugInfo);
 
-void TypeAddObject(Object *object);
+void TypeAddObject(const Object *object);
-void TypeRemoveObject(Object *object);
+void TypeRemoveObject(const Object *object);
 
-void intrusive_ptr_add_ref(Object *object);
+void intrusive_ptr_add_ref(const Object *object);
-void intrusive_ptr_release(Object *object);
+void intrusive_ptr_release(const Object *object);
 
 template<typename T>
 class ObjectImpl

lib/base/shared-object.hpp

@@ -12,8 +12,8 @@ namespace icinga
 
 class SharedObject;
 
-inline void intrusive_ptr_add_ref(SharedObject *object);
+inline void intrusive_ptr_add_ref(const SharedObject *object);
-inline void intrusive_ptr_release(SharedObject *object);
+inline void intrusive_ptr_release(const SharedObject *object);
 
 /**
  * Seamless and polymorphistic base for any class to create shared pointers of.
@@ -23,8 +23,8 @@ inline void intrusive_ptr_release(SharedObject *object);
  */
 class SharedObject
 {
-	friend void intrusive_ptr_add_ref(SharedObject *object);
+	friend void intrusive_ptr_add_ref(const SharedObject *object);
-	friend void intrusive_ptr_release(SharedObject *object);
+	friend void intrusive_ptr_release(const SharedObject *object);
 
 protected:
 	inline SharedObject() : m_References(0)
@@ -38,15 +38,15 @@ protected:
 	~SharedObject() = default;
 
 private:
-	Atomic<uint_fast64_t> m_References;
+	mutable Atomic<uint_fast64_t> m_References;
 };
 
-inline void intrusive_ptr_add_ref(SharedObject *object)
+inline void intrusive_ptr_add_ref(const SharedObject *object)
 {
 	object->m_References.fetch_add(1);
 }
 
-inline void intrusive_ptr_release(SharedObject *object)
+inline void intrusive_ptr_release(const SharedObject *object)
 {
 	if (object->m_References.fetch_sub(1) == 1u) {
 		delete object;

lib/base/shared.hpp

@@ -16,13 +16,13 @@ template<class T>
 class Shared;
 
 template<class T>
-inline void intrusive_ptr_add_ref(Shared<T> *object)
+inline void intrusive_ptr_add_ref(const Shared<T> *object)
 {
 	object->m_References.fetch_add(1);
 }
 
 template<class T>
-inline void intrusive_ptr_release(Shared<T> *object)
+inline void intrusive_ptr_release(const Shared<T> *object)
 {
 	if (object->m_References.fetch_sub(1) == 1u) {
 		delete object;
@@ -38,11 +38,12 @@ inline void intrusive_ptr_release(Shared<T> *object)
 template<class T>
 class Shared : public T
 {
-	friend void intrusive_ptr_add_ref<>(Shared<T> *object);
+	friend void intrusive_ptr_add_ref<>(const Shared<T> *object);
-	friend void intrusive_ptr_release<>(Shared<T> *object);
+	friend void intrusive_ptr_release<>(const Shared<T> *object);
 
 public:
 	typedef boost::intrusive_ptr<Shared> Ptr;
+	typedef boost::intrusive_ptr<const Shared> ConstPtr;
 
 	/**
 	 * Like std::make_shared, but for this class.
@@ -94,7 +95,7 @@ public:
 	}
 
 private:
-	Atomic<uint_fast64_t> m_References;
+	mutable Atomic<uint_fast64_t> m_References;
 };
 
 }

lib/icinga/CMakeLists.txt

@@ -39,7 +39,7 @@ set(icinga_SOURCES
   comment.cpp comment.hpp comment-ti.hpp
   compatutility.cpp compatutility.hpp
   customvarobject.cpp customvarobject.hpp customvarobject-ti.hpp
-  dependency.cpp dependency-group.cpp dependency.hpp dependency-ti.hpp dependency-apply.cpp
+  dependency.cpp dependency-group.cpp dependency-state.cpp dependency.hpp dependency-ti.hpp dependency-apply.cpp
   downtime.cpp downtime.hpp downtime-ti.hpp
   envresolver.cpp envresolver.hpp
   eventcommand.cpp eventcommand.hpp eventcommand-ti.hpp

lib/icinga/checkable-dependency.cpp

@@ -6,14 +6,6 @@
 
 using namespace icinga;
 
-/**
- * The maximum number of allowed dependency recursion levels.
- *
- * This is a subjective limit how deep the dependency tree should be allowed to go, as anything beyond this level
- * is just madness and will likely result in a stack overflow or other undefined behavior.
- */
-static constexpr int l_MaxDependencyRecursionLevel(256);
-
 /**
  * Register all the dependency groups of the current Checkable to the global dependency group registry.
  *
@@ -186,36 +178,15 @@ std::vector<Dependency::Ptr> Checkable::GetReverseDependencies() const
 	return std::vector<Dependency::Ptr>(m_ReverseDependencies.begin(), m_ReverseDependencies.end());
 }
 
-bool Checkable::IsReachable(DependencyType dt, int rstack) const
+/**
+ * Checks whether this checkable is currently reachable according to its dependencies.
+ *
+ * @param dt Dependency type to evaluate for.
+ * @return Whether the given checkable is reachable.
+ */
+bool Checkable::IsReachable(DependencyType dt) const
 {
-	if (rstack > l_MaxDependencyRecursionLevel) {
+	return DependencyStateChecker(dt).IsReachable(this);
-		Log(LogWarning, "Checkable")
-			<< "Too many nested dependencies (>" << l_MaxDependencyRecursionLevel << ") for checkable '" << GetName() << "': Dependency failed.";
-
-		return false;
-	}
-
-	/* implicit dependency on host if this is a service */
-	const auto *service = dynamic_cast<const Service *>(this);
-	if (service && (dt == DependencyState || dt == DependencyNotification)) {
-		Host::Ptr host = service->GetHost();
-
-		if (host && host->GetState() != HostUp && host->GetStateType() == StateTypeHard) {
-			return false;
-		}
-	}
-
-	for (auto& dependencyGroup : GetDependencyGroups()) {
-		if (auto state(dependencyGroup->GetState(this, dt, rstack + 1)); state != DependencyGroup::State::Ok) {
-			Log(LogDebug, "Checkable")
-				<< "Dependency group '" << dependencyGroup->GetRedundancyGroupName() << "' have failed for checkable '"
-				<< GetName() << "': Marking as unreachable.";
-
-			return false;
-		}
-	}
-
-	return true;
 }
 
 /**
@@ -307,9 +278,10 @@ std::set<Checkable::Ptr> Checkable::GetAllChildren() const
  */
 void Checkable::GetAllChildrenInternal(std::set<Checkable::Ptr>& seenChildren, int level) const
 {
-	if (level > l_MaxDependencyRecursionLevel) {
+	if (level > Dependency::MaxDependencyRecursionLevel) {
 		Log(LogWarning, "Checkable")
-			<< "Too many nested dependencies (>" << l_MaxDependencyRecursionLevel << ") for checkable '" << GetName() << "': aborting traversal.";
+			<< "Too many nested dependencies (>" << Dependency::MaxDependencyRecursionLevel << ") for checkable '"
+			<< GetName() << "': aborting traversal.";
 		return;
 	}
 

lib/icinga/checkable.hpp

@@ -84,7 +84,7 @@ public:
 
 	void AddGroup(const String& name);
 
-	bool IsReachable(DependencyType dt = DependencyState, int rstack = 0) const;
+	bool IsReachable(DependencyType dt = DependencyState) const;
 	bool AffectsChildren() const;
 
 	AcknowledgementType GetAcknowledgement();

lib/icinga/dependency-group.cpp

@@ -302,47 +302,10 @@ String DependencyGroup::GetCompositeKey()
  *
  * @param child The child Checkable to evaluate the state for.
  * @param dt The dependency type to evaluate the state for, defaults to DependencyState.
- * @param rstack The recursion stack level to prevent infinite recursion, defaults to 0.
  *
  * @return - Returns the state of the current dependency group.
  */
-DependencyGroup::State DependencyGroup::GetState(const Checkable* child, DependencyType dt, int rstack) const
+DependencyGroup::State DependencyGroup::GetState(const Checkable* child, DependencyType dt) const
 {
-	auto dependencies(GetDependenciesForChild(child));
+	return DependencyStateChecker(dt).GetState(this, child);
-	size_t reachable = 0, available = 0;
-
-	for (const auto& dependency : dependencies) {
-		if (dependency->GetParent()->IsReachable(dt, rstack)) {
-			reachable++;
-
-			// Only reachable parents are considered for availability. If they are unreachable and checks are
-			// disabled, they could be incorrectly treated as available otherwise.
-			if (dependency->IsAvailable(dt)) {
-				available++;
-			}
-		}
-	}
-
-	if (IsRedundancyGroup()) {
-		// The state of a redundancy group is determined by the best state of any parent. If any parent ist reachable,
-		// the redundancy group is reachable, analogously for availability.
-		if (reachable == 0) {
-			return State::Unreachable;
-		} else if (available == 0) {
-			return State::Failed;
-		} else {
-			return State::Ok;
-		}
-	} else {
-		// For dependencies without a redundancy group, dependencies.size() will be 1 in almost all cases. It will only
-		// contain more elements if there are duplicate dependency config objects between two checkables. In this case,
-		// all of them have to be reachable/available as they don't provide redundancy.
-		if (reachable < dependencies.size()) {
-			return State::Unreachable;
-		} else if (available < dependencies.size()) {
-			return State::Failed;
-		} else {
-			return State::Ok;
-		}
-	}
 }

lib/icinga/dependency-state.cpp

@@ -0,0 +1,125 @@
+/* Icinga 2 | (c) 2025 Icinga GmbH | GPLv2+ */
+
+#include "icinga/dependency.hpp"
+#include "icinga/host.hpp"
+#include "icinga/service.hpp"
+
+using namespace icinga;
+
+/**
+ * Construct a helper for evaluating the state of dependencies.
+ *
+ * @param dt Dependency type to check for within the individual methods.
+ */
+DependencyStateChecker::DependencyStateChecker(DependencyType dt)
+	: m_DependencyType(dt)
+{
+}
+
+/**
+ * Checks whether a given checkable is currently reachable.
+ *
+ * @param checkable The checkable to check reachability for.
+ * @param rstack The recursion stack level to prevent infinite recursion, defaults to 0.
+ * @return Whether the given checkable is reachable.
+ */
+bool DependencyStateChecker::IsReachable(Checkable::ConstPtr checkable, int rstack)
+{
+	// If the reachability of this checkable was already computed, return it directly. Otherwise, already create a
+	// temporary map entry that says that this checkable is unreachable so that the different cases returning false
+	// don't have to deal with updating the cache, but only the final return true does. Cyclic dependencies are invalid,
+	// hence recursive calls won't access the potentially not yet correct cached value.
+	if (auto [it, inserted] = m_Cache.insert({checkable, false}); !inserted) {
+		return it->second;
+	}
+
+	if (rstack > Dependency::MaxDependencyRecursionLevel) {
+		Log(LogWarning, "Checkable")
+			<< "Too many nested dependencies (>" << Dependency::MaxDependencyRecursionLevel << ") for checkable '"
+			<< checkable->GetName() << "': Dependency failed.";
+
+		return false;
+	}
+
+	/* implicit dependency on host if this is a service */
+	const auto* service = dynamic_cast<const Service*>(checkable.get());
+	if (service && (m_DependencyType == DependencyState || m_DependencyType == DependencyNotification)) {
+		Host::Ptr host = service->GetHost();
+
+		if (host && host->GetState() != HostUp && host->GetStateType() == StateTypeHard) {
+			return false;
+		}
+	}
+
+	for (auto& dependencyGroup : checkable->GetDependencyGroups()) {
+		if (auto state(GetState(dependencyGroup, checkable.get(), rstack + 1)); state != DependencyGroup::State::Ok) {
+			Log(LogDebug, "Checkable")
+				<< "Dependency group '" << dependencyGroup->GetRedundancyGroupName() << "' have failed for checkable '"
+				<< checkable->GetName() << "': Marking as unreachable.";
+
+			return false;
+		}
+	}
+
+	// Note: This must do the map lookup again. The iterator from above must not be used as a m_Cache.insert() inside a
+	// recursive may have invalidated it.
+	m_Cache[checkable] = true;
+	return true;
+}
+
+/**
+ * Retrieve the state of the given dependency group.
+ *
+ * The state of the dependency group is determined based on the state of the parent Checkables and dependency objects
+ * of the group. A dependency group is considered unreachable when none of the parent Checkables is reachable. However,
+ * a dependency group may still be marked as failed even when it has reachable parent Checkables, but an unreachable
+ * group has always a failed state.
+ *
+ * @param group
+ * @param child The child Checkable to evaluate the state for.
+ * @param rstack The recursion stack level to prevent infinite recursion, defaults to 0.
+ *
+ * @return - Returns the state of the current dependency group.
+ */
+DependencyGroup::State DependencyStateChecker::GetState(const DependencyGroup::ConstPtr& group, const Checkable* child, int rstack)
+{
+	using State = DependencyGroup::State;
+
+	auto dependencies(group->GetDependenciesForChild(child));
+	size_t reachable = 0, available = 0;
+
+	for (const auto& dependency : dependencies) {
+		if (IsReachable(dependency->GetParent(), rstack)) {
+			reachable++;
+
+			// Only reachable parents are considered for availability. If they are unreachable and checks are
+			// disabled, they could be incorrectly treated as available otherwise.
+			if (dependency->IsAvailable(m_DependencyType)) {
+				available++;
+			}
+		}
+	}
+
+	if (group->IsRedundancyGroup()) {
+		// The state of a redundancy group is determined by the best state of any parent. If any parent ist reachable,
+		// the redundancy group is reachable, analogously for availability.
+		if (reachable == 0) {
+			return State::Unreachable;
+		} else if (available == 0) {
+			return State::Failed;
+		} else {
+			return State::Ok;
+		}
+	} else {
+		// For dependencies without a redundancy group, dependencies.size() will be 1 in almost all cases. It will only
+		// contain more elements if there are duplicate dependency config objects between two checkables. In this case,
+		// all of them have to be reachable/available as they don't provide redundancy.
+		if (reachable < dependencies.size()) {
+			return State::Unreachable;
+		} else if (available < dependencies.size()) {
+			return State::Failed;
+		} else {
+			return State::Ok;
+		}
+	}
+}

lib/icinga/dependency.hpp

@@ -49,6 +49,14 @@ public:
 	void SetParent(intrusive_ptr<Checkable> parent);
 	void SetChild(intrusive_ptr<Checkable> child);
 
+	/**
+	 * The maximum number of allowed dependency recursion levels.
+	 *
+	 * This is a subjective limit how deep the dependency tree should be allowed to go, as anything beyond this level
+	 * is just madness and will likely result in a stack overflow or other undefined behavior.
+	 */
+	static constexpr int MaxDependencyRecursionLevel{256};
+
 protected:
 	void OnConfigLoaded() override;
 	void OnAllConfigLoaded() override;
@@ -164,7 +172,7 @@ public:
 	String GetCompositeKey();
 
 	enum class State { Ok, Failed, Unreachable };
-	State GetState(const Checkable* child, DependencyType dt = DependencyState, int rstack = 0) const;
+	State GetState(const Checkable* child, DependencyType dt = DependencyState) const;
 
 	static boost::signals2::signal<void(const Checkable::Ptr&, const DependencyGroup::Ptr&)> OnChildRegistered;
 	static boost::signals2::signal<void(const DependencyGroup::Ptr&, const std::vector<Dependency::Ptr>&, bool)> OnChildRemoved;
@@ -222,6 +230,29 @@ private:
 	static RegistryType m_Registry;
 };
 
+/**
+ * Helper class to evaluate the reachability of checkables and state of dependency groups.
+ *
+ * This class is used for implementing Checkable::IsReachable() and DependencyGroup::GetState().
+ * For this, both methods call each other, traversing the dependency graph recursively. In order
+ * to achieve linear runtime in the graph size, the class internally caches state information
+ * (otherwise, evaluating the state of the same checkable multiple times can result in exponential
+ * worst-case complexity). Because of this cached information is not invalidated, the object is
+ * intended to be short-lived.
+ */
+class DependencyStateChecker
+{
+public:
+	explicit DependencyStateChecker(DependencyType dt);
+
+	bool IsReachable(Checkable::ConstPtr checkable, int rstack = 0);
+	DependencyGroup::State GetState(const DependencyGroup::ConstPtr& group, const Checkable* child, int rstack = 0);
+
+private:
+	DependencyType m_DependencyType;
+	std::unordered_map<Checkable::ConstPtr, bool> m_Cache;
+};
+
 }
 
 #endif /* DEPENDENCY_H */