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 @@ class Object
 	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 @@ class Object
 
 	friend struct ObjectLock;
 
-	friend void intrusive_ptr_add_ref(Object *object);
-	friend void intrusive_ptr_release(Object *object);
+	friend void intrusive_ptr_add_ref(const 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 TypeRemoveObject(Object *object);
+void TypeAddObject(const Object *object);
+void TypeRemoveObject(const Object *object);
 
-void intrusive_ptr_add_ref(Object *object);
-void intrusive_ptr_release(Object *object);
+void intrusive_ptr_add_ref(const 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_release(SharedObject *object);
+inline void intrusive_ptr_add_ref(const 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_release(SharedObject *object);
+	friend void intrusive_ptr_add_ref(const SharedObject *object);
+	friend void intrusive_ptr_release(const SharedObject *object);
 
 protected:
 	inline SharedObject() : m_References(0)
@@ -38,15 +38,15 @@ class SharedObject
 	~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_release<>(Shared<T> *object);
+	friend void intrusive_ptr_add_ref<>(const 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 @@ class Shared : public T
 	}
 
 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) {
-		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;
+	return DependencyStateChecker(dt).IsReachable(this);
 }
 
 /**
@@ -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 @@ class Checkable : public ObjectImpl<Checkable>
 
 	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));
-	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;
-		}
-	}
+	return DependencyStateChecker(dt).GetState(this, child);
 }