From 1c700db3b1c7ea8315db35435d468309f2f90e07 Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Sun, 16 Aug 2026 09:15:50 +0300 Subject: [PATCH 1/3] tests(bridge): cover assignHandlerPrimary's never-attached-binding guard Every existing assignHandlerPrimary test calls ensureBound() first, so currentId is always non-zero going in -- the "raw == 0U" arm of the early-return guard (raw == 0U || primary.empty() || !binding->primary.empty()) was never driven true. A handler that has never attached anything yet has no instance to promote a key onto, so this must return immediately with no assignPrimaryAsync dispatch. Also documents 2 catch(...) blocks (attachHandlerAsync's out-of-frame and in-frame success paths) as genuinely untestable without an OOM-injection allocator seam: the only way in is a std::string copy-assignment throwing std::bad_alloc (currentId.store() on an atomic cannot throw), and forcing that portably is disproportionate for two defensive catches. --- include/morph/core/bridge.hpp | 12 ++++++++++++ tests/test_async_registration.cpp | 24 ++++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/include/morph/core/bridge.hpp b/include/morph/core/bridge.hpp index 6f392a73..cff2c34e 100644 --- a/include/morph/core/bridge.hpp +++ b/include/morph/core/bridge.hpp @@ -564,6 +564,14 @@ class Bridge { "attach reply arrived from a backend switchBackend() already replaced")); } else { try { + // Not covered by this file's own test suite: + // the only way in is a std::string copy- + // assignment throwing (std::bad_alloc from a + // real allocation failure -- currentId.store() + // on an atomic cannot throw). Forcing that + // portably needs an OOM-injection allocator + // seam this codebase does not have, and is + // disproportionate for one defensive catch. strongBinding->contextKey = primaryCopy; strongBinding->primary = primaryCopy; strongBinding->currentId.store(newId.v); @@ -599,6 +607,10 @@ class Bridge { std::exception_ptr failure = parked->failure; if (parked->succeeded) { try { + // Not covered by this file's own test suite: same + // OOM-only catch as attachHandlerAsync's out-of-frame + // callback above -- see its comment for why this is + // left uncovered rather than forced. binding->contextKey = primaryCopy; binding->primary = std::move(primaryCopy); binding->currentId.store(parked->modelId.v); diff --git a/tests/test_async_registration.cpp b/tests/test_async_registration.cpp index 6741dd8b..80e82899 100644 --- a/tests/test_async_registration.cpp +++ b/tests/test_async_registration.cpp @@ -1057,6 +1057,30 @@ TEST_CASE( CHECK(binding->contextKey == "999"); } +TEST_CASE("Bridge::assignHandlerPrimary: a never-attached binding (currentId still 0) is a silent no-op", + "[bridge][registration][issue67]") { + // Every other assignHandlerPrimary test in this file calls ensureBound() + // first, so currentId is always non-zero going in -- the `raw == 0U` + // arm of the early-return guard (raw == 0U || primary.empty() || + // !binding->primary.empty()) is otherwise never driven true. A handler + // that has never attached anything yet has no instance to promote a key + // onto, so this must return immediately: no assignPrimaryAsync dispatch, + // no pendingCount bump. + morph::exec::ThreadPoolExecutor pool{2}; + auto backend = std::make_unique(pool); + auto* rawBackend = backend.get(); + morph::bridge::Bridge bridge{std::move(backend)}; + + auto binding = std::make_shared(); + binding->typeId = "AR_CreateModel"; + binding->modelFactory = [] { return morph::model::detail::ModelFactory::create(); }; + REQUIRE(binding->currentId.load() == 0U); // never bound + + bridge.assignHandlerPrimary(binding, "100"); + CHECK(rawBackend->pendingCount() == 0); + CHECK(binding->primary.empty()); +} + TEST_CASE( "Bridge::assignHandlerPrimary: a stale async reply after the binding itself is dropped is a safe no-op", "[bridge][registration][issue67]") { From 2b7c9faaf25cd18d85af83a017b60b69966d9497 Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Sun, 16 Aug 2026 09:37:53 +0300 Subject: [PATCH 2/3] docs(bridge): file morph#108 for the OOM-injection seam these 2 catches need Per feedback: prefer a trackable issue over a comment alone for gaps that genuinely need a library addition to close, matching the existing morph#95/#96/#97 pattern already used elsewhere in this codebase. --- include/morph/core/bridge.hpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/include/morph/core/bridge.hpp b/include/morph/core/bridge.hpp index cff2c34e..a16e9002 100644 --- a/include/morph/core/bridge.hpp +++ b/include/morph/core/bridge.hpp @@ -570,8 +570,10 @@ class Bridge { // real allocation failure -- currentId.store() // on an atomic cannot throw). Forcing that // portably needs an OOM-injection allocator - // seam this codebase does not have, and is - // disproportionate for one defensive catch. + // seam this codebase does not have (see + // LASTRADA-Software/morph#108, requesting + // one). Left uncovered rather than + // exhausting process memory in a test. strongBinding->contextKey = primaryCopy; strongBinding->primary = primaryCopy; strongBinding->currentId.store(newId.v); From b2ebbb7daf40845ca4259a894171c92a545e8c33 Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Sun, 16 Aug 2026 10:12:08 +0300 Subject: [PATCH 3/3] bridge: drop in-code OOM-untested comments now that morph#108 tracks it The two catch (...) blocks in attachHandlerAsync's out-of-frame and in-frame success paths no longer carry an inline comment explaining they're OOM-only and untested -- morph#108 already tracks exactly this (the allocator seam needed to test them), so the explanation lives there instead of duplicated at each call site. Co-Authored-By: Claude Sonnet 5 --- include/morph/core/bridge.hpp | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/include/morph/core/bridge.hpp b/include/morph/core/bridge.hpp index a16e9002..6f392a73 100644 --- a/include/morph/core/bridge.hpp +++ b/include/morph/core/bridge.hpp @@ -564,16 +564,6 @@ class Bridge { "attach reply arrived from a backend switchBackend() already replaced")); } else { try { - // Not covered by this file's own test suite: - // the only way in is a std::string copy- - // assignment throwing (std::bad_alloc from a - // real allocation failure -- currentId.store() - // on an atomic cannot throw). Forcing that - // portably needs an OOM-injection allocator - // seam this codebase does not have (see - // LASTRADA-Software/morph#108, requesting - // one). Left uncovered rather than - // exhausting process memory in a test. strongBinding->contextKey = primaryCopy; strongBinding->primary = primaryCopy; strongBinding->currentId.store(newId.v); @@ -609,10 +599,6 @@ class Bridge { std::exception_ptr failure = parked->failure; if (parked->succeeded) { try { - // Not covered by this file's own test suite: same - // OOM-only catch as attachHandlerAsync's out-of-frame - // callback above -- see its comment for why this is - // left uncovered rather than forced. binding->contextKey = primaryCopy; binding->primary = std::move(primaryCopy); binding->currentId.store(parked->modelId.v);