From 63e9ef58ba6a0ed941b5d7389a436aa9dbe26046 Mon Sep 17 00:00:00 2001 From: Julian Brost Date: Mon, 28 Jul 2025 15:32:05 +0200 Subject: [PATCH] Prevent worst-case exponential complexity in dependency evaluation So far, calling Checkable::IsReachable() traversed all possible paths to it's parents. In case a parent is reachable via multiple paths, all it's parents were evaluated multiple times, result in a worst-case exponential complexity. With this commit, the implementation keeps track of which checkables were already visited and uses the already-computed reachability instead of repeating the computation, ensuring a worst-case linear runtime within the graph size. --- lib/icinga/dependency-state.cpp | 11 +++++++++++ lib/icinga/dependency.hpp | 1 + 2 files changed, 12 insertions(+) diff --git a/lib/icinga/dependency-state.cpp b/lib/icinga/dependency-state.cpp index 675347cac..cd1548b42 100644 --- a/lib/icinga/dependency-state.cpp +++ b/lib/icinga/dependency-state.cpp @@ -25,6 +25,14 @@ DependencyStateChecker::DependencyStateChecker(DependencyType dt) */ 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 '" @@ -53,6 +61,9 @@ bool DependencyStateChecker::IsReachable(Checkable::ConstPtr checkable, int rsta } } + // 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; } diff --git a/lib/icinga/dependency.hpp b/lib/icinga/dependency.hpp index 1ff9ad387..c70e578b4 100644 --- a/lib/icinga/dependency.hpp +++ b/lib/icinga/dependency.hpp @@ -250,6 +250,7 @@ public: private: DependencyType m_DependencyType; + std::unordered_map m_Cache; }; }