fix: make AlgorithmSvc usable

core/include/algorithms/algorithm.h

@@ -30,10 +30,13 @@
 #pragma once
 
 #include <array>
+#include <functional>
 #include <map>
+#include <memory>
 #include <mutex>
 #include <optional>
 #include <string>
+#include <string_view>
 #include <tuple>
 #include <typeindex>
 #include <typeinfo>
@@ -105,64 +108,63 @@ template <class InputType, class OutputType> class Algorithm : public AlgorithmB
 // compile-time code needed.
 class AlgorithmSvc : public LoggedService<AlgorithmSvc> {
 public:
+  void init() {}
+
   // Add a new factory for an algorithm with a specified type. The full demangled type name
   // will be used as identifier for the factory
   template <class Algo> void add() {
-    static std::mutex m;
-    static std::lock_guard<std::mutex> lock{m};
-    auto type            = Algo::algorithm_type;
-    std::type_index name = detail::demangledName<Algo>();
-    if (!available<type>()) {
-      m_factories[type] = {};
-    }
-    if (available<type>(name)) {
-      return; // do nothing, already there
-    }
-    m_factories[type].emplace({name, []() { return static_cast<AlgorithmBase*>(new Algo()); }});
+    using AlgoType = typename Algo::algorithm_type;
+    std::lock_guard<std::mutex> lock{m_mutex};
+    auto& factories = m_factories[typeid(AlgoType)];
+    // do nothing if already there
+    factories.try_emplace(detail::demangledName<Algo>(),
+                          []() -> std::unique_ptr<AlgorithmBase> { return std::make_unique<Algo>(); });
   }
 
   // Get a new owning pointer to an instance of an algorithm.
   // Throws if the resource isn't available, or if we aren't fully ready yet.
   template <class AlgoType> std::unique_ptr<AlgoType> get(std::string_view name) const {
     ensureReady();
-    if (!available<AlgoType>(name)) {
+    std::lock_guard<std::mutex> lock{m_mutex};
+    const auto* factory = find<AlgoType>(name);
+    if (factory == nullptr) {
       raise(fmt::format("No factory with name {} and type {} registered with the AlgorithmSvc",
-                        name, typeid(AlgoType).name()));
+                        name, detail::demangledName<AlgoType>()));
     }
-    const auto& factories = m_factories.at(typeid(AlgoType));
-    // This creates and object and gives ownership to the unique_ptr
-    return static_cast<AlgoType*>(factories.at(name)());
+    // This creates an object and gives ownership to the unique_ptr
+    return std::unique_ptr<AlgoType>(static_cast<AlgoType*>((*factory)().release()));
   }
   // Return a vector of the names of all available algorithms of a certain type
   // Just return an empty vector if no names are present (don't throw any exceptions
   // for missing entries here).
   // Throws if we aren't fully ready yet.
-  template <class AlgoType> auto ls() const {
+  template <class AlgoType> std::vector<std::string> ls() const {
     ensureReady();
-    std::vector<std::string_view> ret;
-    if (available<AlgoType>()) {
-      std::for_each(factory<AlgoType>().begin(), factory<AlgoType>().end(), std::back_inserter(ret),
-                    [](auto&& key_value_pair) { return key_value_pair.first(); });
+    std::lock_guard<std::mutex> lock{m_mutex};
+    std::vector<std::string> ret;
+    if (const auto it = m_factories.find(typeid(AlgoType)); it != m_factories.end()) {
+      for (const auto& [name, factory] : it->second) {
+        ret.push_back(name);
+      }
     }
     return ret;
   }
 
 private:
-  // Do we have any algorithms available of a certain type?
-  template <class AlgoType> bool available() const { return !m_factories.count(typeid(AlgoType)); }
-  // Do we have any algorithms of type AlgoType with a specific name available? Throws
-  // if any of the lower-level assumptions fail
-  template <class AlgoType> bool available(std::string_view name) const {
-    if (name == "") {
+  using factory_type = std::function<std::unique_ptr<AlgorithmBase>()>;
+
+  // Find the factory of type AlgoType with a specific name, or nullptr if there is none.
+  // Throws if the name is invalid
+  template <class AlgoType> const factory_type* find(std::string_view name) const {
+    if (name.empty()) {
       raise(fmt::format("Invalid name provided: '{}'", name));
     }
-    return factory<AlgoType>().count(name) != 0;
-  }
-  template <class AlgoType> const auto& factory() const {
-    if (!available<AlgoType>()) {
-      raise(fmt::format("No factory for algorithm type {} provided", typeid(AlgoType).name()));
+    const auto it = m_factories.find(typeid(AlgoType));
+    if (it == m_factories.end()) {
+      return nullptr;
     }
-    return m_factories.at(typeid(AlgoType));
+    const auto factory = it->second.find(name);
+    return (factory != it->second.end()) ? &factory->second : nullptr;
   }
   // Throw an exception if we are not ready as factory to avoid giving incomplete factory
   // information to the caller
@@ -172,9 +174,10 @@ class AlgorithmSvc : public LoggedService<AlgorithmSvc> {
     }
   }
 
-  using factory_type = std::function<std::unique_ptr<AlgorithmBase>()>;
+  mutable std::mutex m_mutex;
+  std::map<std::type_index, std::map<std::string, factory_type, std::less<>>> m_factories;
 
-  std::map<std::type_index, std::map<std::string_view, factory_type>> m_factories;
+  ALGORITHMS_DEFINE_LOGGED_SERVICE(AlgorithmSvc)
 };
 
 namespace detail {

core/tests/algorithm_svc.cpp

@@ -0,0 +1,106 @@
+// SPDX-License-Identifier: LGPL-3.0-or-later
+// Copyright (C) 2026 Wouter Deconinck
+//
+// Tests for the AlgorithmSvc factory service
+//
+// The AlgorithmSvc only works after ServiceSvc::init(), which can only be called once per
+// process, so the full lifecycle is tested in a single test case.
+//
+#include <algorithms/algorithm.h>
+
+#include <catch2/catch_test_macros.hpp>
+#include <catch2/matchers/catch_matchers_string.hpp>
+
+#include <string>
+#include <vector>
+
+using namespace algorithms;
+using Catch::Matchers::ContainsSubstring;
+
+namespace algorithm_svc_test {
+
+using IntAlgorithm    = Algorithm<Input<int>, Output<int>>;
+using DoubleAlgorithm = Algorithm<Input<double>, Output<double>>;
+
+class AddOne : public IntAlgorithm {
+public:
+  AddOne() : IntAlgorithm{"AddOne", {"in"}, {"out"}, "Adds one"} {}
+  void process(const Input& input, const Output& output) const final {
+    *std::get<0>(output) = *std::get<0>(input) + 1;
+  }
+};
+
+class Twice : public IntAlgorithm {
+public:
+  Twice() : IntAlgorithm{"Twice", {"in"}, {"out"}, "Multiplies by two"} {}
+  void process(const Input& input, const Output& output) const final {
+    *std::get<0>(output) = *std::get<0>(input) * 2;
+  }
+};
+
+class Half : public DoubleAlgorithm {
+public:
+  Half() : DoubleAlgorithm{"Half", {"in"}, {"out"}, "Divides by two"} {}
+  void process(const Input& input, const Output& output) const final {
+    *std::get<0>(output) = *std::get<0>(input) / 2;
+  }
+};
+
+} // namespace algorithm_svc_test
+
+using namespace algorithm_svc_test;
+
+TEST_CASE("AlgorithmSvc creates registered algorithms by type and name", "[algorithm]") {
+  auto& svc = AlgorithmSvc::instance();
+  CHECK(svc.name() == "AlgorithmSvc");
+
+  svc.add<AddOne>();
+  svc.add<Twice>();
+  svc.add<Half>();
+  // adding an algorithm twice has no effect
+  svc.add<AddOne>();
+
+  // the service cannot be used before all services are initialized
+  CHECK_THROWS_AS(svc.ls<IntAlgorithm>(), Error);
+  CHECK_THROWS_WITH(svc.get<IntAlgorithm>("algorithm_svc_test::AddOne"),
+                    "Attempt to use AlgorithmSvc, but service not yet marked as ready");
+
+  ServiceSvc::instance().init();
+  REQUIRE(svc.ready());
+
+  // algorithms are listed by type, with their demangled type name
+  CHECK(svc.ls<IntAlgorithm>() ==
+        std::vector<std::string>{"algorithm_svc_test::AddOne", "algorithm_svc_test::Twice"});
+  CHECK(svc.ls<DoubleAlgorithm>() == std::vector<std::string>{"algorithm_svc_test::Half"});
+  CHECK(svc.ls<Algorithm<Input<float>, Output<float>>>().empty());
+
+  // each get creates a new instance
+  const auto add_one = svc.get<IntAlgorithm>("algorithm_svc_test::AddOne");
+  const auto twice   = svc.get<IntAlgorithm>("algorithm_svc_test::Twice");
+  const auto half    = svc.get<DoubleAlgorithm>("algorithm_svc_test::Half");
+  REQUIRE(add_one != nullptr);
+  REQUIRE(twice != nullptr);
+  REQUIRE(half != nullptr);
+  CHECK(add_one != svc.get<IntAlgorithm>("algorithm_svc_test::AddOne"));
+  CHECK(add_one->name() == "AddOne");
+  CHECK(twice->name() == "Twice");
+
+  const int i = 3;
+  int j       = 0;
+  add_one->process({&i}, {&j});
+  CHECK(j == 4);
+  twice->process({&i}, {&j});
+  CHECK(j == 6);
+
+  const double x = 3.;
+  double y       = 0.;
+  half->process({&x}, {&y});
+  CHECK(y == 1.5);
+
+  // unknown names, types and invalid names are rejected
+  CHECK_THROWS_WITH(svc.get<IntAlgorithm>("algorithm_svc_test::Half"),
+                    ContainsSubstring("No factory with name algorithm_svc_test::Half and type"));
+  CHECK_THROWS_WITH((svc.get<Algorithm<Input<float>, Output<float>>>("AddOne")),
+                    ContainsSubstring("No factory with name AddOne"));
+  CHECK_THROWS_WITH(svc.get<IntAlgorithm>(""), "Invalid name provided: ''");
+}