From 4b28e4626e744ac9b779a680f8e9647014956dda Mon Sep 17 00:00:00 2001
From: Felix Morgner <felix.morgner@ost.ch>
Date: Sun, 26 Jul 2026 21:46:37 +0200
Subject: kapi/devices: implement locking discipline

---
 kernel/kapi/devices/bus.cpp             |  38 +++++-----
 kernel/kapi/devices/bus.stress.cpp      |  76 +++++++++++++++++++
 kernel/kapi/devices/device_registry.cpp |  96 +++++++++++++----------
 kernel/kapi/devices/facet_registry.cpp  | 130 ++++++++++++++++++++------------
 4 files changed, 233 insertions(+), 107 deletions(-)
 create mode 100644 kernel/kapi/devices/bus.stress.cpp

(limited to 'kernel/kapi')

diff --git a/kernel/kapi/devices/bus.cpp b/kernel/kapi/devices/bus.cpp
index d917eb96..71dd01e6 100644
--- a/kernel/kapi/devices/bus.cpp
+++ b/kernel/kapi/devices/bus.cpp
@@ -6,6 +6,7 @@
 #include <kapi/system.hpp>
 
 #include <kstd/memory.hpp>
+#include <kstd/mutex.hpp>
 #include <kstd/string.hpp>
 #include <kstd/vector.hpp>
 
@@ -42,30 +43,32 @@ namespace kapi::devices
       kapi::system::panic("[OS:DEV] Failed to register child device {}", child->name());
     }
 
-    // TODO: lock bus
+    auto attached = kstd::shared_ptr<device>{};
+    {
+      auto guard = kstd::lock_guard{m_lock};
+      attached = m_devices.emplace_back(std::move(child));
+    }
 
-    auto & attached = m_devices.emplace_back(std::move(child));
     attached->set_state(state::present);
 
-    // TODO: unlock bus
-
     driver_registry::get().device_attached(attached);
   }
 
   auto bus::remove_child(device & device) -> bool  // NOLINT(misc-no-recursion)
   {
-    // TODO: lock bus
-
-    auto found = std::ranges::find_if(m_devices, [&](auto const & d) { return d.get() == &device; });
-    if (!found)
+    auto lifeline = kstd::shared_ptr<struct device>{};
     {
-      return false;
-    }
+      auto guard = kstd::lock_guard{m_lock};
 
-    auto lifeline = *found;
-    m_devices.erase(found);
+      auto found = std::ranges::find_if(m_devices, [&](auto const & d) { return d.get() == &device; });
+      if (found == m_devices.end())
+      {
+        return false;
+      }
 
-    // TODO: unlock bus
+      lifeline = *found;
+      m_devices.erase(found);
+    }
 
     do_remove_child(*lifeline);
     return true;
@@ -100,10 +103,11 @@ namespace kapi::devices
     if (auto child_bus = device.facet<bus>())
     {
       auto grandchildren = kstd::vector<kstd::shared_ptr<kapi::devices::device>>{};
-      // TODO: lock child bus
-      // NOTE: this could be more efficient one vector::assign is implemented.
-      grandchildren = kstd::vector(child_bus->m_devices.begin(), child_bus->m_devices.end());
-      // TODO: unlock child bus
+      {
+        auto guard = kstd::lock_guard{m_lock};
+        // NOTE: this could be more efficient one vector::assign is implemented.
+        grandchildren = kstd::vector(child_bus->m_devices.begin(), child_bus->m_devices.end());
+      }
 
       for (auto & grandchild : grandchildren)
       {
diff --git a/kernel/kapi/devices/bus.stress.cpp b/kernel/kapi/devices/bus.stress.cpp
new file mode 100644
index 00000000..226b0296
--- /dev/null
+++ b/kernel/kapi/devices/bus.stress.cpp
@@ -0,0 +1,76 @@
+#include <kapi/devices.hpp>
+
+#include <kstd/format.hpp>
+#include <kstd/memory.hpp>
+
+#include <catch2/catch_test_macros.hpp>
+
+#include <atomic>
+#include <cstddef>
+#include <thread>
+#include <tuple>
+#include <vector>
+
+namespace
+{
+
+  struct test_device final : kapi::devices::device
+  {
+    using kapi::devices::device::device;
+  };
+
+}  // namespace
+
+constexpr auto thread_count = 32;
+constexpr auto devices_per_thread = 200;
+
+SCENARIO("Concurrent attach/detach/lookup on a bus is race-free")
+{
+  GIVEN("A bus shared by several threads")
+  {
+    auto shared_bus = kstd::make_shared<kapi::devices::bus>("stress_test_bus");
+    kapi::devices::get_root_bus()->add_child(shared_bus);
+
+    WHEN("each thread repeatedly attaches, detaches, and looks up devices concurrently with the others")
+    {
+      auto threads = std::vector<std::jthread>{};
+      threads.reserve(thread_count);
+
+      auto failure_count = std::atomic<std::size_t>{0};
+
+      for (auto thread_index = 0uz; thread_index < thread_count; ++thread_index)
+      {
+        threads.emplace_back([&shared_bus, &failure_count, thread_index] {
+          for (auto i = 0uz; i < devices_per_thread; ++i)
+          {
+            auto name = kstd::format("stress_test_device_{}_{}", thread_index, i);
+            auto device = kstd::make_shared<test_device>(name);
+
+            shared_bus->add_child(device);
+
+            std::ignore = kapi::devices::device_registry::get().find(name);
+            std::ignore = kapi::devices::device_registry::get().all();
+            std::ignore = shared_bus->children().size();
+
+            if (!shared_bus->remove_child(*device))
+            {
+              ++failure_count;
+            };
+          }
+        });
+      }
+
+      threads.clear();
+
+      THEN("every attach was matched by a successful removal")
+      {
+        REQUIRE(failure_count == 0);
+      }
+
+      THEN("the bus has no children")
+      {
+        REQUIRE(shared_bus->children().empty());
+      }
+    }
+  }
+}
\ No newline at end of file
diff --git a/kernel/kapi/devices/device_registry.cpp b/kernel/kapi/devices/device_registry.cpp
index f5c20a6d..80ddde65 100644
--- a/kernel/kapi/devices/device_registry.cpp
+++ b/kernel/kapi/devices/device_registry.cpp
@@ -4,6 +4,7 @@
 #include <kapi/system.hpp>
 
 #include <kstd/memory.hpp>
+#include <kstd/mutex.hpp>
 #include <kstd/print.hpp>
 #include <kstd/string.hpp>
 #include <kstd/vector.hpp>
@@ -27,7 +28,7 @@ namespace kapi::devices
       system::panic("[OS:DEV] Device registry has already been initialized");
     }
 
-    instance = device_registry{};
+    instance.emplace();
   }
 
   auto device_registry::get() -> device_registry &
@@ -50,33 +51,38 @@ namespace kapi::devices
     kstd::println("[OS:DEV] Registering device {}", device->name());
 
     auto added = false;
-
-    // TODO: lock the registry
-    auto found = m_devices.find(device->name());
-    if (found != m_devices.end())
     {
-      if (!found->second.expired())
+      auto guard = kstd::lock_guard{m_lock};
+
+      auto found = m_devices.find(device->name());
+      if (found != m_devices.end())
       {
-        added = false;
-      }
+        if (!found->second.expired())
+        {
+          added = false;
+        }
 
-      found->second = device;
-      added = true;
-    }
-    else
-    {
-      added = m_devices.emplace(device->name(), device).second;
+        found->second = device;
+        added = true;
+      }
+      else
+      {
+        added = m_devices.emplace(device->name(), device).second;
+      }
     }
-    // TODO: unlock registry
 
     if (!added)
     {
       return false;
     }
 
-    // TODO: consider if we need to lock the registry observers here, since somebody may add an observer in the meantime
-    // once multiple codepaths can run at the same time.
-    for (auto const & observer : m_observers)
+    auto observers = kstd::vector<kstd::weak_ptr<device_registry_observer>>{};
+    {
+      auto guard = kstd::lock_guard{m_lock};
+      observers = m_observers;
+    }
+
+    for (auto const & observer : observers)
     {
       if (auto locked_observer = observer.lock())
       {
@@ -84,36 +90,43 @@ namespace kapi::devices
       }
     }
 
-    // TODO: lock registry
-    erase_if(m_observers, [](auto const & observer) { return observer.expired(); });
-    // TODO: unlock registry
+    {
+      auto guard = kstd::lock_guard{m_lock};
+      erase_if(m_observers, [](auto const & observer) { return observer.expired(); });
+    }
 
     return true;
   }
 
   auto device_registry::remove(device & device) -> bool
   {
-    // TODO: lock registry
-
-    auto found = m_devices.find(device.name());
-    if (found == m_devices.end())
     {
-      return false;
-    }
+      auto guard = kstd::lock_guard{m_lock};
 
-    auto locked = found->second.lock();
-    if (!locked || locked.get() != &device)
-    {
-      return false;
-    }
+      auto found = m_devices.find(device.name());
+      if (found == m_devices.end())
+      {
+        return false;
+      }
 
-    found->second = kstd::weak_ptr<struct device>{};
+      auto locked = found->second.lock();
+      if (!locked || locked.get() != &device)
+      {
+        return false;
+      }
 
-    // TODO: unlock registry
+      found->second = kstd::weak_ptr<struct device>{};
+    }
 
     kstd::println("[OS:DEV] Unregistering device {}", device.name());
 
-    for (auto const & observer : m_observers)
+    auto observers = kstd::vector<kstd::weak_ptr<device_registry_observer>>{};
+    {
+      auto guard = kstd::lock_guard{m_lock};
+      observers = m_observers;
+    }
+
+    for (auto const & observer : observers)
     {
       if (auto locked_observer = observer.lock())
       {
@@ -121,16 +134,17 @@ namespace kapi::devices
       }
     }
 
-    // TODO: lock registry
-    erase_if(m_observers, [](auto const & observer) { return observer.expired(); });
-    // TODO: unlock registry
+    {
+      auto guard = kstd::lock_guard{m_lock};
+      erase_if(m_observers, [](auto const & observer) { return observer.expired(); });
+    }
 
     return true;
   }
 
   auto device_registry::find(std::string_view name) const -> kstd::shared_ptr<device>
   {
-    // TODO: lock registry
+    auto guard = kstd::lock_guard{m_lock};
 
     auto found = m_devices.find(kstd::string{name});
     if (found == m_devices.end())
@@ -143,7 +157,7 @@ namespace kapi::devices
 
   auto device_registry::all() const -> kstd::vector<kstd::shared_ptr<device>>
   {
-    // TODO: lock registry
+    auto guard = kstd::lock_guard{m_lock};
 
     auto result = kstd::vector<kstd::shared_ptr<device>>{};
 
@@ -160,7 +174,7 @@ namespace kapi::devices
 
   auto device_registry::subscribe(kstd::weak_ptr<device_registry_observer> observer) -> void
   {
-    // TODO: lock registry
+    auto guard = kstd::lock_guard{m_lock};
 
     erase_if(m_observers, [](auto const & observer) { return observer.expired(); });
     m_observers.push_back(observer);
diff --git a/kernel/kapi/devices/facet_registry.cpp b/kernel/kapi/devices/facet_registry.cpp
index 4aeebfc1..623ecba2 100644
--- a/kernel/kapi/devices/facet_registry.cpp
+++ b/kernel/kapi/devices/facet_registry.cpp
@@ -5,6 +5,7 @@
 #include <kapi/test_support/devices.hpp>
 
 #include <kstd/memory.hpp>
+#include <kstd/mutex.hpp>
 #include <kstd/result.hpp>
 #include <kstd/string.hpp>
 #include <kstd/system_error.hpp>
@@ -46,64 +47,87 @@ namespace kapi::devices
   auto facet_registry::do_publish(kstd::shared_ptr<device> device, kstd::string name, kapi::capabilities::facet_id id,
                                   void * facet) -> kstd::result<void>
   {
-    // TODO: lock registry
+    auto published = std::optional<entry>{};
 
-    erase_if(m_entries, [id](auto e) { return e.id() == id && !e.device(); });
-
-    if (!device || !facet || name.empty())
     {
-      return kstd::failure(make_error_code(kstd::errc::invalid_argument));
-    }
+      auto guard = kstd::lock_guard{m_lock};
 
-    auto already_published = std::ranges::any_of(
-        m_entries, [&](auto const & entry) { return entry.id() == id && entry.device().get() == device.get(); });
+      erase_if(m_entries, [id](auto e) { return e.id() == id && !e.device(); });
 
-    if (already_published)
-    {
-      return kstd::failure(make_error_code(kstd::errc::file_exists));
-    }
+      if (!device || !facet || name.empty())
+      {
+        return kstd::failure(make_error_code(kstd::errc::invalid_argument));
+      }
 
-    auto & published = m_entries.emplace_back(device, std::move(name), id, facet);
+      auto already_published = std::ranges::any_of(
+          m_entries, [&](auto const & entry) { return entry.id() == id && entry.device().get() == device.get(); });
 
-    do_notify_published(id, published);
+      if (already_published)
+      {
+        return kstd::failure(make_error_code(kstd::errc::file_exists));
+      }
+
+      published = m_entries.emplace_back(device, std::move(name), id, facet);
+    }
+
+    do_notify_published(id, *published);
 
     return kstd::success();
   }
 
   auto facet_registry::do_notify_published(kapi::capabilities::facet_id id, entry const & published) -> void
   {
-    std::ranges::for_each(m_observers, [&](auto observer) {
+    auto observers = kstd::vector<kstd::weak_ptr<facet_registry_observer>>{};
+    auto static_observers = kstd::vector<facet_registry_observer *>{};
+    {
+      auto guard = kstd::lock_guard{m_lock};
+      observers = m_observers;
+      static_observers = m_static_observers;
+    }
+
+    std::ranges::for_each(observers, [&](auto observer) {
       if (auto locked_observer = observer.lock())
       {
         locked_observer->on_facet_published(id, published);
       }
     });
 
-    std::ranges::for_each(m_static_observers, [&](auto observer) { observer->on_facet_published(id, published); });
+    std::ranges::for_each(static_observers, [&](auto observer) { observer->on_facet_published(id, published); });
   }
 
   auto facet_registry::do_notify_withdrawn(kapi::capabilities::facet_id id, device const & device) -> void
   {
-    std::ranges::for_each(m_observers, [&](auto observer) {
+    auto observers = kstd::vector<kstd::weak_ptr<facet_registry_observer>>{};
+    auto static_observers = kstd::vector<facet_registry_observer *>{};
+    {
+      auto guard = kstd::lock_guard{m_lock};
+      observers = m_observers;
+      static_observers = m_static_observers;
+    }
+
+    std::ranges::for_each(observers, [&](auto observer) {
       if (auto locked_observer = observer.lock())
       {
         locked_observer->on_facet_withdrawn(id, const_cast<devices::device &>(device));
       }
     });
 
-    std::ranges::for_each(m_static_observers, [&](auto observer) {
+    std::ranges::for_each(static_observers, [&](auto observer) {
       observer->on_facet_withdrawn(id, const_cast<devices::device &>(device));
     });
   }
 
   auto facet_registry::withdraw(device const & device, kapi::capabilities::facet_id id) -> void
   {
-    // TODO: lock registry
+    auto did_remove = false;
 
-    auto did_remove = erase_if(m_entries, [&](auto e) {
-                        auto locked_device = e.device();
-                        return e.id() == id && locked_device && locked_device.get() == &device;
-                      }) != 0;
+    {
+      auto guard = kstd::lock_guard{m_lock};
+      did_remove = erase_if(m_entries, [&](auto e) {
+                     auto locked_device = e.device();
+                     return e.id() == id && locked_device && locked_device.get() == &device;
+                   }) != 0;
+    }
 
     if (did_remove)
     {
@@ -113,26 +137,28 @@ namespace kapi::devices
 
   auto facet_registry::withdraw_all_for(device const & device) -> void
   {
-    // TODO: lock registry
-
     auto withdrawn_facets = kstd::vector<kapi::capabilities::facet_id>{};
 
-    erase_if(m_entries, [&](auto e) {
-      auto locked_device = e.device();
-      auto do_erase = locked_device && locked_device.get() == &device;
-      if (do_erase)
-      {
-        withdrawn_facets.push_back(e.id());
-      }
-      return do_erase;
-    });
+    {
+      auto guard = kstd::lock_guard{m_lock};
+
+      erase_if(m_entries, [&](auto e) {
+        auto locked_device = e.device();
+        auto do_erase = locked_device && locked_device.get() == &device;
+        if (do_erase)
+        {
+          withdrawn_facets.push_back(e.id());
+        }
+        return do_erase;
+      });
+    }
 
     std::ranges::for_each(withdrawn_facets, [&](auto facet) { do_notify_withdrawn(facet, device); });
   }
 
   auto facet_registry::all(kapi::capabilities::facet_id id) const -> kstd::vector<entry>
   {
-    // TODO: lock registry
+    auto guard = kstd::lock_guard{m_lock};
 
     auto filtered = m_entries;
     erase_if(filtered, [&](auto e) {
@@ -142,33 +168,39 @@ namespace kapi::devices
     return filtered;
   }
 
-  auto facet_registry::resolve(kapi::capabilities::facet_id id, std::string_view name) -> void *
+  auto facet_registry::resolve(kapi::capabilities::facet_id facet, std::string_view name) -> void *
   {
-    // TODO: lock registry
+    auto found_device = kstd::shared_ptr<device>{};
+    {
+      auto guard = kstd::lock_guard{m_lock};
 
-    auto found = std::ranges::find_if(m_entries, [&](auto e) { return e.name() == name && e.device(); });
+      auto found = std::ranges::find_if(m_entries, [&](auto e) { return e.name() == name && e.device(); });
 
-    if (found == m_entries.cend())
-    {
-      return nullptr;
+      if (found == m_entries.cend())
+      {
+        return nullptr;
+      }
+
+      found_device = found->device();
     }
-    else if (auto device = found->device())
+
+    if (!found_device)
     {
-      return resolve(id, *device);
+      return nullptr;
     }
 
-    return nullptr;
+    return resolve(facet, *found_device);
   }
 
   auto facet_registry::resolve(kapi::capabilities::facet_id id, device & device) -> void *
   {
-    // TODO: lock registry
-
     if (auto by_device = device.facet(id))
     {
       return by_device;
     }
 
+    auto guard = kstd::lock_guard{m_lock};
+
     auto found = std::ranges::find_if(m_entries, [&](auto e) {
       auto locked_device = e.device();
       return e.id() == id && locked_device && locked_device.get() == &device;
@@ -184,7 +216,7 @@ namespace kapi::devices
 
   auto facet_registry::subscribe(kstd::weak_ptr<facet_registry_observer> observer) -> void
   {
-    // TODO: lock registry
+    auto guard = kstd::lock_guard{m_lock};
 
     erase_if(m_observers, [](auto const & observer) { return observer.expired(); });
     m_observers.push_back(observer);
@@ -192,14 +224,14 @@ namespace kapi::devices
 
   auto facet_registry::subscribe(facet_registry_observer & observer) -> void
   {
-    // TODO: lock registry
+    auto guard = kstd::lock_guard{m_lock};
 
     m_static_observers.push_back(&observer);
   }
 
   auto facet_registry::unsubscribe(facet_registry_observer & observer) -> void
   {
-    // TODO: lock registry
+    auto guard = kstd::lock_guard{m_lock};
 
     erase_if(m_observers, [&](auto const & subscribed) { return subscribed.lock().get() == &observer; });
     erase(m_static_observers, &observer);
-- 
cgit v1.2.3