Repository navigation
rewrite(phase2): port watchdog + build allocation fault-injection seam, ported from c-RCP - #133
Merged
Conversation
…m, ported from c-RCP Part of the v3.0.0 ground-up rewrite (cpp-RCP #129, ROADMAP.md Phase 17), branch rewrite/v3-from-c-rcp. First batch of Phase 2, alongside e2e/ lifecycle (separate PR). watchdog.hpp: investigation established cpp-RCP's StreamWatchdog/Manager is not a stale transliteration target -- it's a deliberate, already- shipped v2.10.0 design (ROADMAP.md milestone 54) with its own REQ-WDG-001..008 catalog that doesn't correspond 1:1 to c-RCP's REQ-WDG-001..012. c-RCP's own watchdog has since evolved to a background-thread rcp_watchdog_keeper_t (a later c-RCP redesign, not what this codebase's design was ever meant to mirror) -- porting that would contradict this codebase's own stated 'no clock/thread of its own' convention and break sim.hpp/mdns.hpp's existing dependents. Treated as a content audit instead, mapping each of c-RCP's 12 REQ-WDG-* against what transfers vs. doesn't. One genuine gap found and fixed: the c-RCP-17 fixed-capacity conversion (RCP_WATCHDOG_MAX_STREAMS /RCP_WATCHDOG_MAX_CALLBACKS, both 16) had never been ported -- Manager used unbounded std::map/std::vector, unlike every other Phase 17 module. Converted to fixed std::array-backed storage with swap-removal (matching loan.hpp's free-list technique); register_stream/subscribe now return std::error_code (new stream_capacity_exceeded/ callback_capacity_exceeded) instead of silently discarding overflow. alloc.hpp (NEW): cpp-RCP had zero allocation-failure-injection capability -- confirmed zero try/catch(std::bad_alloc) anywhere in the tree. c-RCP's own mechanism (a global rcp_alloc_set_hooks() function- pointer table) is deliberately NOT ported literally -- it's exactly the pattern that needed a later access-control retrofit in c-RCP itself (issue #600/PR #615, rcp_alloc_lock_hooks()) after any caller could silently redirect the allocator backing safety code. Built a small, instance-owned rcp::alloc::FaultInjector instead (mutex-guarded, arm(count)/should_fail()), matching faultinject.hpp's existing idiom. Phase 1's fixed-capacity modules (request/fragment/respqueue/loan) were checked first and found to not need it -- their capacity-exceeded paths are std::array-backed and never touch the heap, so they're already directly testable and can't be meaningfully fault-injected. Wired FaultInjector into loan.hpp's BufferPool::loan() cache-miss branch instead (its one real heap allocation), as a genuine, non-artificial demonstration it composes with a real caller -- optional constructor parameter, defaulted nullptr, fully backward-compatible. REQ-ALLOC-* (11 in c-RCP) breakdown, documented in alloc.hpp's header: none transfer as written -- 001/002/010/011 describe the global hook table this design deliberately lacks; 003-006 describe C's four separate allocator entry points, no C++ equivalent without a global operator new override; 007-009 (lock/unlock/query) exist only to retrofit access control onto a table that doesn't exist here. The one concept that transfers -- deterministic, portable allocation-failure testability -- is what FaultInjector::should_fail() provides. Call sites touched: sim.hpp's Simulator::register_stream widened from void to std::error_code to propagate the new capacity-exceeded result (existing callers discard it as a statement, no break). test_watchdog: 12 -> 15 test cases, 137 assertions. test_alloc (new): 13 test cases, 98 assertions. Verified: full tree rebuilds clean (223/223 targets), 56/56 ctest suites pass (100%) -- independently re-verified from a fresh build, not just the porting agent's own report. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Matt <47545907+SoundMatt@users.noreply.github.com>
10 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of the v3.0.0 ground-up rewrite (#129, ROADMAP.md Phase 17). First batch of Phase 2, alongside
e2e/lifecycle(separate PR).watchdog.hppInvestigation established cpp-RCP's
StreamWatchdog/Manageris not a stale transliteration target — it's a deliberate, already-shipped v2.10.0 design with its ownREQ-WDG-001..008catalog that doesn't map 1:1 to c-RCP'sREQ-WDG-001..012. c-RCP's own watchdog has since evolved to a background-thread design that would contradict this codebase's "no clock/thread of its own" convention and breaksim.hpp/mdns.hpp's dependents — correctly not ported. Treated as a content audit instead. One genuine gap found and fixed: the c-RCP-17 fixed-capacity conversion (RCP_WATCHDOG_MAX_STREAMS/MAX_CALLBACKS, both 16) had never landed here —Managerused unboundedstd::map/std::vector, unlike every other Phase 17 module. Converted to fixedstd::array-backed storage;register_stream/subscribenow returnstd::error_codeon overflow instead of silently discarding it.alloc.hpp(NEW)cpp-RCP had zero allocation-failure-injection capability. c-RCP's own mechanism (a global function-pointer hook table) is deliberately not ported literally — it's exactly the pattern c-RCP itself needed an access-control retrofit for later (issue #600/PR #615). Built a small, instance-owned
FaultInjectorinstead, matchingfaultinject.hpp's existing idiom. Phase 1's fixed-capacity modules were checked first and found not to need it (their overflow paths never touch the heap); wired it intoloan.hpp'sBufferPool::loan()cache-miss branch instead — its one real heap allocation — as a genuine demonstration, optional/backward-compatible.REQ-ALLOC-*(11 in c-RCP): none transfer as written (documented breakdown in the header) — the one concept that does, deterministic allocation-failure testability, is whatFaultInjectorprovides.Call site touched:
sim.hpp'sSimulator::register_streamwidenedvoid→std::error_codeto propagate the new capacity result (no break, existing callers discard it as a statement).test_watchdog: 12→15 cases.test_alloc(new): 13 cases.Verified
Full tree rebuilds clean (223/223 targets), 56/56 ctest suites pass (100%) — independently re-verified from a fresh build.
Tracked at #129.