Protect against uninitialized services (#666)

elbeno · web-flow · commit 9702670cc6d0 · 2025-01-01T18:38:00.000-08:00
* 🎨 Use injected class name in `nexus`

* 🦺 Protect against uninitialized services

Problem:
- If services are uninitialized, either through not being exported properly, or
  through `nexus.init()` not being called, the service pointers are null and the
  resulting UB causes hard-to-diagnose effects.

Solution:
- Initialize services to functions that cause a runtime panic.
diff --git a/include/cib/builder_meta.hpp b/include/cib/builder_meta.hpp
@@ -1,10 +1,13 @@
 #pragma once
 
+#include <concepts>
+
 namespace cib {
 template <typename T>
 concept builder_meta = requires {
     typename T::builder_t;
     typename T::interface_t;
+    { T::uninitialized() } -> std::same_as<typename T::interface_t>;
 };
 
 template <builder_meta T> using builder_t = typename T::builder_t;
diff --git a/include/cib/built.hpp b/include/cib/built.hpp
@@ -3,5 +3,6 @@
 #include <cib/builder_meta.hpp>
 
 namespace cib {
-template <builder_meta ServiceMeta> interface_t<ServiceMeta> service;
+template <builder_meta ServiceMeta>
+constinit auto service = ServiceMeta::uninitialized();
 } // namespace cib
diff --git a/include/cib/callback.hpp b/include/cib/callback.hpp
@@ -3,6 +3,7 @@
 #include <cib/builder_meta.hpp>
 
 #include <stdx/compiler.hpp>
+#include <stdx/panic.hpp>
 
 #include <array>
 #include <concepts>
@@ -119,5 +120,12 @@ template <int NumFuncs = 0, typename... ArgTypes> struct builder {
 template <typename... ArgTypes> struct service {
     using builder_t = builder<0, ArgTypes...>;
     using interface_t = void (*)(ArgTypes...);
+
+    CONSTEVAL static auto uninitialized() -> interface_t {
+        return [](ArgTypes...) {
+            stdx::panic<
+                "Attempting to run callback before it is initialized">();
+        };
+    }
 };
 } // namespace callback
diff --git a/include/cib/nexus.hpp b/include/cib/nexus.hpp
@@ -16,25 +16,22 @@ namespace cib {
  *
  * @see cib::config
  */
-template <typename Config> struct nexus {
-  private:
-    using this_t = nexus<Config>;
 
+template <typename Config> struct nexus {
 // Workaround unfortunate bug in clang where it can't deduce "auto" sometimes
 #define CIB_BUILD_SERVICE                                                      \
     initialized<Config, Tag>::value.template build<initialized<Config, Tag>>()
 
-  public:
     template <typename Tag>
     constexpr static decltype(CIB_BUILD_SERVICE) service = CIB_BUILD_SERVICE;
 #undef CIB_BUILD_SERVICE
 
     static void init() {
         auto const service = []<typename T> {
-            using from_t = std::remove_cvref_t<decltype(this_t::service<T>)>;
+            using from_t = std::remove_cvref_t<decltype(nexus::service<T>)>;
             using to_t = std::remove_cvref_t<decltype(cib::service<T>)>;
 
-            auto &service_impl = this_t::service<T>;
+            auto &service_impl = nexus::service<T>;
             if constexpr (std::is_convertible_v<from_t, to_t>) {
                 cib::service<T> = service_impl;
             } else {
diff --git a/include/flow/builder.hpp b/include/flow/builder.hpp
@@ -4,7 +4,9 @@
 #include <flow/graph_builder.hpp>
 #include <flow/impl.hpp>
 
+#include <stdx/compiler.hpp>
 #include <stdx/ct_string.hpp>
+#include <stdx/panic.hpp>
 
 namespace flow {
 template <stdx::ct_string Name = "">
@@ -13,5 +15,13 @@ using builder = graph<Name, graph_builder<Name, impl>>;
 template <stdx::ct_string Name = ""> struct service {
     using builder_t = builder<Name>;
     using interface_t = FunctionPtr;
+
+    CONSTEVAL static auto uninitialized() -> interface_t {
+        return [] {
+            using namespace stdx::literals;
+            stdx::panic<"Attempting to run flow ("_cts + Name +
+                        ") before it is initialized"_cts>();
+        };
+    }
 };
 } // namespace flow
diff --git a/include/msg/handler_interface.hpp b/include/msg/handler_interface.hpp
@@ -1,5 +1,11 @@
 #pragma once
 
+#include <stdx/compiler.hpp>
+#include <stdx/ct_conversions.hpp>
+#include <stdx/ct_string.hpp>
+#include <stdx/panic.hpp>
+#include <stdx/type_traits.hpp>
+
 namespace msg {
 template <typename MsgBase, typename... ExtraCallbackArgs>
 struct handler_interface {
@@ -8,4 +14,33 @@ struct handler_interface {
     virtual auto handle(MsgBase const &msg,
                         ExtraCallbackArgs... extra_args) const -> bool = 0;
 };
+
+namespace detail {
+template <typename M>
+concept named_msg_base =
+    stdx::is_specialization_of<decltype(M::name), stdx::ct_string>().value;
+
+template <typename M> CONSTEVAL auto name_for_msg() {
+    if constexpr (detail::named_msg_base<M>) {
+        return M::name;
+    } else {
+        constexpr auto name = stdx::type_as_string<M>();
+        return stdx::ct_string<name.size() + 1>{name};
+    }
+}
+} // namespace detail
+
+template <typename MsgBase, typename... ExtraCallbackArgs>
+struct uninitialized_handler_t
+    : handler_interface<MsgBase, ExtraCallbackArgs...> {
+    auto is_match(MsgBase const &) const -> bool override { return false; }
+
+    auto handle(MsgBase const &, ExtraCallbackArgs...) const -> bool override {
+        using namespace stdx::literals;
+        stdx::panic<"Attempting to handle msg ("_cts +
+                    detail::name_for_msg<MsgBase>() +
+                    ") before service is initialized"_cts>();
+        return false;
+    }
+};
 } // namespace msg
diff --git a/include/msg/indexed_service.hpp b/include/msg/indexed_service.hpp
@@ -3,6 +3,7 @@
 #include <msg/handler_interface.hpp>
 #include <msg/indexed_builder.hpp>
 
+#include <stdx/compiler.hpp>
 #include <stdx/tuple.hpp>
 
 namespace msg {
@@ -12,5 +13,11 @@ struct indexed_service {
                                       ExtraCallbackArgs...>;
     using interface_t =
         handler_interface<MsgBase, ExtraCallbackArgs...> const *;
+
+    constexpr static auto uninitialized_v =
+        uninitialized_handler_t<MsgBase, ExtraCallbackArgs...>{};
+    CONSTEVAL static auto uninitialized() -> interface_t {
+        return &uninitialized_v;
+    }
 };
 } // namespace msg
diff --git a/include/msg/message.hpp b/include/msg/message.hpp
@@ -349,6 +349,8 @@ template <stdx::ct_string Name, typename... Fields> struct message {
         (... and Fields::template fits_inside<S>());
 
     template <typename T> struct base {
+        constexpr static auto name = Name;
+
         constexpr auto as_derived() const -> T const & {
             return static_cast<T const &>(*this);
         }
diff --git a/include/msg/service.hpp b/include/msg/service.hpp
@@ -3,13 +3,19 @@
 #include <msg/handler_builder.hpp>
 #include <msg/handler_interface.hpp>
 
-#include <stdx/tuple.hpp>
+#include <stdx/compiler.hpp>
 
 namespace msg {
 template <typename MsgBase, typename... ExtraCallbackArgs> struct service {
     using builder_t =
         handler_builder<stdx::tuple<>, MsgBase, ExtraCallbackArgs...>;
     using interface_t =
         handler_interface<MsgBase, ExtraCallbackArgs...> const *;
+
+    constexpr static auto uninitialized_v =
+        uninitialized_handler_t<MsgBase, ExtraCallbackArgs...>{};
+    CONSTEVAL static auto uninitialized() -> interface_t {
+        return &uninitialized_v;
+    }
 };
 } // namespace msg
diff --git a/test/cib/CMakeLists.txt b/test/cib/CMakeLists.txt
@@ -2,6 +2,7 @@ add_tests(
     FILES
     builder_meta
     callback
+    callback_uninit
     nexus
     readme_hello_world
     LIBRARIES
diff --git a/test/cib/builder_meta.cpp b/test/cib/builder_meta.cpp
@@ -1,16 +1,19 @@
 #include <cib/builder_meta.hpp>
 
+#include <stdx/compiler.hpp>
+
 #include <catch2/catch_test_macros.hpp>
 
 #include <type_traits>
 
 namespace {
-struct TestBuilderTag;
-struct TestInterfaceTag;
+struct TestBuilder;
+struct TestInterface {};
 
 struct test_builder_meta {
-    using builder_t = TestBuilderTag;
-    using interface_t = TestInterfaceTag;
+    using builder_t = TestBuilder;
+    using interface_t = TestInterface;
+    CONSTEVAL static auto uninitialized() -> interface_t;
 };
 } // namespace
 
@@ -21,7 +24,7 @@ TEST_CASE("builder_meta concept") {
 TEST_CASE(
     "builder_meta builder and interface type traits return correct values") {
     static_assert(
-        std::is_same_v<TestBuilderTag, cib::builder_t<test_builder_meta>>);
+        std::is_same_v<TestBuilder, cib::builder_t<test_builder_meta>>);
     static_assert(
-        std::is_same_v<TestInterfaceTag, cib::interface_t<test_builder_meta>>);
+        std::is_same_v<TestInterface, cib::interface_t<test_builder_meta>>);
 }
diff --git a/test/cib/callback_uninit.cpp b/test/cib/callback_uninit.cpp
@@ -0,0 +1,35 @@
+#include <cib/built.hpp>
+#include <cib/callback.hpp>
+
+#include <stdx/panic.hpp>
+
+#include <catch2/catch_test_macros.hpp>
+
+#include <string>
+#include <string_view>
+
+namespace {
+template <int Id, typename... Args>
+struct TestCallback : public callback::service<Args...> {};
+
+std::string panic_string = {};
+int panics{};
+
+struct test_panic_handler {
+    template <stdx::ct_string Why, typename... Ts>
+    static auto panic(Ts &&...) -> void {
+        panic_string = std::string_view{Why};
+        ++panics;
+    }
+};
+} // namespace
+
+template <> inline auto stdx::panic_handler<> = test_panic_handler{};
+
+TEST_CASE("run callback service when uninitialized", "[callback]") {
+    panics = 0;
+    cib::service<TestCallback<0>>();
+    CHECK(panics == 1);
+    CHECK(panic_string ==
+          "Attempting to run callback before it is initialized");
+}
diff --git a/test/flow/CMakeLists.txt b/test/flow/CMakeLists.txt
@@ -11,6 +11,7 @@ add_unit_test(
 add_tests(
     FILES
     flow
+    flow_uninit
     graph
     graph_builder
     logging
diff --git a/test/flow/flow.cpp b/test/flow/flow.cpp
@@ -54,6 +54,7 @@ using alt_builder = flow::graph<Name, flow::graphviz_builder>;
 template <stdx::ct_string Name = ""> struct alt_flow_service {
     using builder_t = alt_builder<Name>;
     using interface_t = flow::VizFunctionPtr;
+    constexpr static auto uninitialized() -> interface_t { return {}; }
 };
 
 struct VizFlow : public alt_flow_service<"debug"> {};
diff --git a/test/flow/flow_uninit.cpp b/test/flow/flow_uninit.cpp
@@ -0,0 +1,34 @@
+#include <cib/built.hpp>
+#include <flow/builder.hpp>
+
+#include <stdx/panic.hpp>
+
+#include <catch2/catch_test_macros.hpp>
+
+#include <string>
+#include <string_view>
+
+namespace {
+struct TestFlow : flow::service<"test"> {};
+
+std::string panic_string = {};
+int panics{};
+
+struct test_panic_handler {
+    template <stdx::ct_string Why, typename... Ts>
+    static auto panic(Ts &&...) -> void {
+        panic_string = std::string_view{Why};
+        ++panics;
+    }
+};
+} // namespace
+
+template <> inline auto stdx::panic_handler<> = test_panic_handler{};
+
+TEST_CASE("run flow when uninitialized", "[flow]") {
+    panics = 0;
+    cib::service<TestFlow>();
+    CHECK(panics == 1);
+    CHECK(panic_string ==
+          "Attempting to run flow (test) before it is initialized");
+}
diff --git a/test/msg/CMakeLists.txt b/test/msg/CMakeLists.txt
@@ -6,9 +6,11 @@ add_tests(
     field_matchers
     handler
     handler_builder
+    handler_uninit
     indexed_builder
     indexed_callback
     indexed_handler
+    indexed_handler_uninit
     message
     send
     LIBRARIES
diff --git a/test/msg/handler_uninit.cpp b/test/msg/handler_uninit.cpp
@@ -0,0 +1,54 @@
+#include <cib/built.hpp>
+#include <msg/message.hpp>
+#include <msg/service.hpp>
+
+#include <stdx/panic.hpp>
+
+#include <catch2/catch_test_macros.hpp>
+
+#include <cstdint>
+#include <string>
+#include <string_view>
+
+struct raw_msg_t {};
+
+namespace {
+using namespace msg;
+
+using msg_defn = message<"msg">;
+using test_msg_t = msg::owning<msg_defn>;
+using msg_view_t = msg::const_view<msg_defn>;
+struct test_service : msg::service<msg_view_t> {};
+struct raw_service : msg::service<raw_msg_t> {};
+
+std::string panic_string = {};
+int panics{};
+
+struct test_panic_handler {
+    template <stdx::ct_string Why, typename... Ts>
+    static auto panic(Ts &&...) -> void {
+        panic_string = std::string_view{Why};
+        ++panics;
+    }
+};
+} // namespace
+
+template <> inline auto stdx::panic_handler<> = test_panic_handler{};
+
+TEST_CASE("invoke handle on service when uninitialized (named msg type)",
+          "[handler]") {
+    panics = 0;
+    cib::service<test_service>->handle(test_msg_t{});
+    CHECK(panics == 1);
+    CHECK(panic_string ==
+          "Attempting to handle msg (msg) before service is initialized");
+}
+
+TEST_CASE("invoke handle on service when uninitialized (raw msg type)",
+          "[handler]") {
+    panics = 0;
+    cib::service<raw_service>->handle(raw_msg_t{});
+    CHECK(panics == 1);
+    CHECK(panic_string ==
+          "Attempting to handle msg (raw_msg_t) before service is initialized");
+}
diff --git a/test/msg/indexed_handler_uninit.cpp b/test/msg/indexed_handler_uninit.cpp

Original file line number	Diff line number	Diff line change
`@@ -349,6 +349,8 @@ template <stdx::ct_string Name, typename... Fields> struct message {`
`349`	`349`	`(... and Fields::template fits_inside<S>());`
`350`	`350`
`351`	`351`	`template <typename T> struct base {`
	`352`	`+ constexpr static auto name = Name;`
	`353`	`+`
`352`	`354`	`constexpr auto as_derived() const -> T const & {`
`353`	`355`	`return static_cast<T const &>(*this);`
`354`	`356`	`}`