Skip to content

rewrite(phase5): shmem.hpp — rebuild Channel around a real bounded byte-level buffer - #153

Merged
SoundMatt merged 2 commits into
rewrite/v3-from-c-rcpfrom
phase5/shmem
Aug 22, 2026
Merged

rewrite(phase5): shmem.hpp — rebuild Channel around a real bounded byte-level buffer#153
SoundMatt merged 2 commits into
rewrite/v3-from-c-rcpfrom
phase5/shmem

Conversation

@SoundMatt

@SoundMatt SoundMatt commented Aug 22, 2026

Copy link
Copy Markdown
Owner

Part of the v3.0.0 ground-up rewrite (#129, ROADMAP.md Phase 17), branch rewrite/v3-from-c-rcp. Phase 5 (transport layer), Wave 2.

Architecture decision (from a dedicated investigation this session)

cpp-RCP's shmem.hpp was a decoded-object pass-through — no byte encoding, no capacity bound. c-RCP's own reference is a real byte-level bounded ring-buffer transport, but is ALSO in-process only under the hood (plain heap, non-pshared mutex/condvar) — neither codebase has ever had genuine cross-process shared memory. Decision: rebuild Channel around a real bounded byte-level buffer (closing the two real gaps — no capacity bound, codec never exercised) while keeping the existing decoded-object Handler signature at the boundary, matching udp::Server's own internal-codec/external-decoded-callback shape. admin.hpp/config.hpp need zero changes — confirmed by reading both in full.

Ported

RCP_ERR_BUSY-equivalent backpressure via a new fixed-capacity detail::FrameSlots, real acf::encode/decode_acf_abb/gbb on both request and response legs (previously never exercised), and the runtime queue_capacity parameter (clamped [1, kMaxQueueCapacity]).

Bug found and fixed mid-task

The original code held the handler mutex across the handler call itself, which would serialize every concurrent request and deadlock under the new capacity tests — fixed by copying the handler out under the lock, then invoking it unlocked.

Verification

shmem.hpp: 173→458 lines. test_shmem.cpp: 170→526 lines, 20 TEST_CASEs (68 assertions), re-run 5x back-to-back with no flakiness in the new thread/condvar-based backpressure tests.

Full tree rebuilds clean (0 errors/warnings), 58/58 ctest suites pass (100%) — independently re-verified from a fresh build.

SoundMatt and others added 2 commits August 22, 2026 02:28
@SoundMatt @claude
rewrite(phase5): shmem.hpp — rebuild Channel around a real bounded by…
6858060 
…te-level buffer

Part of the v3.0.0 ground-up rewrite (cpp-RCP #129, ROADMAP.md Phase 17),
branch rewrite/v3-from-c-rcp. Phase 5 (transport layer), Wave 2.

A dedicated investigation (this session) found cpp-RCP's shmem.hpp was
a decoded-object pass-through with no byte encoding at all and no
capacity bound -- Channel::request() called a std::function handler
directly and synchronously in the same process. c-RCP's
rcp_shmem_avtp_pair_new() is a real byte-level bounded ring-buffer
transport, real memcpy framing, RCP_ERR_BUSY backpressure when full --
but is ALSO in-process only under the hood (plain heap, non-pshared
mutex/condvar). Neither codebase has ever had genuine cross-process
shared memory; "shmem" means "a fast paired in-process queue" in both
trees. Decision: rebuild Channel around a real byte-level bounded
buffer (closing the two real gaps -- no capacity bound, codec never
exercised) while keeping the existing decoded-object Handler signature
at the boundary (matching udp::Server's own internal-codec/external-
decoded-callback shape), so admin.hpp/config.hpp need zero changes.

## Ported (c-RCP src/shmem.c 266 lines / include/rcp/shmem.h 99 lines)

- shmem.c:68-71 (RCP_ERR_BUSY on full queue) -> Channel's new
  detail::FrameSlots<kMaxQueueCapacity> bounded pool + ErrBusy return,
  REQ-SHMEM-006.
- shmem.c:64-67/109-113 (closed-side send/recv RCP_ERR_CLOSED) ->
  Channel::request()'s existing closed_ check, now cited REQ-SHMEM-005/010.
- The codec-never-exercised gap: Channel::request() now genuinely
  calls acf::encode_acf_abb/encode_acf_gbb and decode_acf_abb/
  decode_acf_gbb (selection mirrors udp.hpp's encode_frame/decode_frame)
  on both the request and response legs before/after the Handler runs.
- shmem.h:91-93's runtime queue_capacity parameter (clamped to min 1,
  shmem.c:204) -> Channel(stream_key, queue_capacity =
  kDefaultQueueCapacity), clamped into [1, kMaxQueueCapacity].

## New shape

Channel keeps its old Handler signature and synchronous, no-thread-
of-its-own dispatch model exactly. Internally, request() now: encodes
-> acquires a fixed-capacity slot (ErrBusy if none free) -> decodes
back out of that slot -> calls Handler with decoded objects (handler
copied out from under a separate mutex and invoked UNLOCKED, so
concurrent callers' handlers genuinely run in parallel) -> encodes/
decodes the response -> releases the slot via RAII. Registry's public
surface is byte-for-byte unchanged -- confirmed by reading all of
admin.hpp/config.hpp/test_admin.cpp/test_config.cpp in full: they only
use Registry::add_channel/deregister/lookup/channels/close and
Channel::stream_key()/new_channel(), never Channel::request()'s
internals. No changes needed to any of the four.

## Bug found and fixed mid-task (real concurrency bug)

The original code held the handler mutex ACROSS the handler call
itself, which would have serialized every concurrent request and
deadlocked under the new capacity tests -- fixed by copying the
handler out under the lock, then invoking it unlocked.

## Architecture note (documented in the header)

Unlike c-RCP's genuinely two-party, strict-FIFO ring (two independent
threads holding *out_a/*out_b), Channel::request() is both producer
and consumer in one synchronous call, so the bounded buffer is a
per-caller indexed slot pool, not a literal head/tail FIFO -- same
capacity bound and BUSY-without-enqueuing contract, no independent-
reader ordering to preserve since there's only ever one reader.

## REQ-SHMEM mapping / c-RCP citation drift found (not propagated)

.fusa-reqs.json was split 2026-08-18 (issue #533) into REQ-SHMEM-001..
010 (not 001..009). Found three req-id mis-citations in c-RCP's own
src/shmem.c inline comments (shmem_side_send citing 007 instead of
006; shmem_side_recv wrongly also citing 006; shmem_side_destroy
citing 007 instead of 008) -- c-RCP's own test_shmem.c citations are
correct, only the .c file's comments drifted; not propagated here.
REQ-SHMEM-004 (recv timeout) and REQ-SHMEM-007 (recv buffer-too-small)
have no analog in the new cpp shape, documented as such rather than
force-fit.

## Verification

include/rcp/shmem.hpp: 173 -> 458 lines. tests/test_shmem.cpp: 170 ->
526 lines, test_shmem: 20 TEST_CASEs (68 assertions), re-run 5x
back-to-back with no flakiness in the new thread/condition-variable-
based backpressure tests. Full tree rebuilds clean (0 errors, 0
warnings), 58/58 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>
@SoundMatt @claude
fix(shmem): correct REQ-SHMEM-004/007 test coverage — numbering mix-u…
e97b00d 
…p, not a real gap

The prior commit's header comment claimed REQ-SHMEM-004/007 had 'no
analog' in the new architecture, based on c-RCP's own shmem.h numbering
(where those ids mean recv() timeout / recv() destination-buffer-too-
small). That was a numbering mix-up, not an actual gap: THIS project's
.fusa-reqs.json assigns REQ-SHMEM-004 to 'Registry::lookup finds a
channel registered via add_channel' and REQ-SHMEM-007 to 'the
caller-supplied client id is forwarded to the handler unchanged' --
both ordinary, already-implemented, directly testable behaviors.

- REQ-SHMEM-004: the existing 'Registry::lookup finds a channel
  registered via add_channel' test already covered this exact
  behavior -- it was just missing the requirement tag ([registry]
  only). Added the tag.
- REQ-SHMEM-007: no test existed for client-id forwarding at all.
  Added one, asserting the handler sees exactly the client id passed
  to request() across two different values (not cached/defaulted from
  a prior call).

Fixes the cpfusa trace (100%% coverage) gate failure on PR #153
(tested=410/total=412 -- these were the two untested entries).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Matt <47545907+SoundMatt@users.noreply.github.com>
@SoundMatt
SoundMatt merged commit 017051b into rewrite/v3-from-c-rcp Aug 22, 2026
22 checks passed
@SoundMatt
SoundMatt deleted the phase5/shmem branch August 22, 2026 09:42
@SoundMatt SoundMatt mentioned this pull request Aug 22, 2026
Merged
SoundMatt added a commit that referenced this pull request Aug 22, 2026
@SoundMatt @claude
rewrite(phase5): admin.hpp — fixed-capacity subscriber/counter bounds…
4db668c 
…, port deadlock fix (#154)

* rewrite(phase5): shmem.hpp — rebuild Channel around a real bounded byte-level buffer

Part of the v3.0.0 ground-up rewrite (cpp-RCP #129, ROADMAP.md Phase 17),
branch rewrite/v3-from-c-rcp. Phase 5 (transport layer), Wave 2.

A dedicated investigation (this session) found cpp-RCP's shmem.hpp was
a decoded-object pass-through with no byte encoding at all and no
capacity bound -- Channel::request() called a std::function handler
directly and synchronously in the same process. c-RCP's
rcp_shmem_avtp_pair_new() is a real byte-level bounded ring-buffer
transport, real memcpy framing, RCP_ERR_BUSY backpressure when full --
but is ALSO in-process only under the hood (plain heap, non-pshared
mutex/condvar). Neither codebase has ever had genuine cross-process
shared memory; "shmem" means "a fast paired in-process queue" in both
trees. Decision: rebuild Channel around a real byte-level bounded
buffer (closing the two real gaps -- no capacity bound, codec never
exercised) while keeping the existing decoded-object Handler signature
at the boundary (matching udp::Server's own internal-codec/external-
decoded-callback shape), so admin.hpp/config.hpp need zero changes.

## Ported (c-RCP src/shmem.c 266 lines / include/rcp/shmem.h 99 lines)

- shmem.c:68-71 (RCP_ERR_BUSY on full queue) -> Channel's new
  detail::FrameSlots<kMaxQueueCapacity> bounded pool + ErrBusy return,
  REQ-SHMEM-006.
- shmem.c:64-67/109-113 (closed-side send/recv RCP_ERR_CLOSED) ->
  Channel::request()'s existing closed_ check, now cited REQ-SHMEM-005/010.
- The codec-never-exercised gap: Channel::request() now genuinely
  calls acf::encode_acf_abb/encode_acf_gbb and decode_acf_abb/
  decode_acf_gbb (selection mirrors udp.hpp's encode_frame/decode_frame)
  on both the request and response legs before/after the Handler runs.
- shmem.h:91-93's runtime queue_capacity parameter (clamped to min 1,
  shmem.c:204) -> Channel(stream_key, queue_capacity =
  kDefaultQueueCapacity), clamped into [1, kMaxQueueCapacity].

## New shape

Channel keeps its old Handler signature and synchronous, no-thread-
of-its-own dispatch model exactly. Internally, request() now: encodes
-> acquires a fixed-capacity slot (ErrBusy if none free) -> decodes
back out of that slot -> calls Handler with decoded objects (handler
copied out from under a separate mutex and invoked UNLOCKED, so
concurrent callers' handlers genuinely run in parallel) -> encodes/
decodes the response -> releases the slot via RAII. Registry's public
surface is byte-for-byte unchanged -- confirmed by reading all of
admin.hpp/config.hpp/test_admin.cpp/test_config.cpp in full: they only
use Registry::add_channel/deregister/lookup/channels/close and
Channel::stream_key()/new_channel(), never Channel::request()'s
internals. No changes needed to any of the four.

## Bug found and fixed mid-task (real concurrency bug)

The original code held the handler mutex ACROSS the handler call
itself, which would have serialized every concurrent request and
deadlocked under the new capacity tests -- fixed by copying the
handler out under the lock, then invoking it unlocked.

## Architecture note (documented in the header)

Unlike c-RCP's genuinely two-party, strict-FIFO ring (two independent
threads holding *out_a/*out_b), Channel::request() is both producer
and consumer in one synchronous call, so the bounded buffer is a
per-caller indexed slot pool, not a literal head/tail FIFO -- same
capacity bound and BUSY-without-enqueuing contract, no independent-
reader ordering to preserve since there's only ever one reader.

## REQ-SHMEM mapping / c-RCP citation drift found (not propagated)

.fusa-reqs.json was split 2026-08-18 (issue #533) into REQ-SHMEM-001..
010 (not 001..009). Found three req-id mis-citations in c-RCP's own
src/shmem.c inline comments (shmem_side_send citing 007 instead of
006; shmem_side_recv wrongly also citing 006; shmem_side_destroy
citing 007 instead of 008) -- c-RCP's own test_shmem.c citations are
correct, only the .c file's comments drifted; not propagated here.
REQ-SHMEM-004 (recv timeout) and REQ-SHMEM-007 (recv buffer-too-small)
have no analog in the new cpp shape, documented as such rather than
force-fit.

## Verification

include/rcp/shmem.hpp: 173 -> 458 lines. tests/test_shmem.cpp: 170 ->
526 lines, test_shmem: 20 TEST_CASEs (68 assertions), re-run 5x
back-to-back with no flakiness in the new thread/condition-variable-
based backpressure tests. Full tree rebuilds clean (0 errors, 0
warnings), 58/58 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>

* rewrite(phase5): admin.hpp — fixed-capacity subscriber/counter bounds, port deadlock fix

Part of the v3.0.0 ground-up rewrite (cpp-RCP #129, ROADMAP.md Phase 17),
branch rewrite/v3-from-c-rcp. Phase 5 (transport layer), final module.

## Ported (c-RCP src/admin.c 247 lines / include/rcp/admin.h 177 lines)

- admin.c:109-110 (RCP_ADMIN_MAX_SUBSCRIBERS=16, RCP_ADMIN_MAX_COUNTERS=256)
  -> std::array<..., kMaxSubscribers>/std::array<..., kMaxCounters> bounds
  on Server's subscriber list and counter table, replacing cpp-RCP's
  unbounded std::vector/std::map. Matches this rewrite's fixed-capacity,
  no-dynamic-allocation-on-safety-relevant-paths architecture (already
  applied to server.hpp's admission queue and shmem.hpp's FrameSlots).
- admin.c:133-147 (rcp_admin_server_subscribe, capacity check before
  add) -> Server::subscribe() returns AdminErrc::subscriber_capacity_exceeded
  once kMaxSubscribers is reached, matching c-RCP's fail-closed "cb not
  added" behavior (admin.h:145) rather than growing unbounded.
- admin.c:173-198 (rcp_admin_server_record_counter, capacity check on a
  genuinely new counter name+labels pair) -> counter_capacity_exceeded
  once kMaxCounters distinct counters exist (admin.h:158), existing
  counters still update past the bound.
- Ported c-RCP's two boundary tests (subscribe-at-capacity,
  record_counter-at-capacity) into Catch2 TEST_CASEs.

## Bug found and fixed (real concurrency bug, same class as shmem.hpp/#153)

emit() (ported from rcp_admin_server_emit(), admin.c:149-171) held the
subscriber-list mutex across every subscriber callback invocation --
a subscriber callback that itself calls back into Server (a realistic
pattern for a metrics/event sink) would deadlock. Fixed by copying the
subscriber list out under the lock, then invoking callbacks unlocked --
same fix shape as shmem.hpp's Channel::request() handler-invocation fix
in #153.

## Deliberate non-port, documented in-file

c-RCP's admin.c is caller-driven: rcp_registry_t was retired project-
wide (per c-RCP's own history) and every admin_server_t call site now
passes state in directly rather than looking it up from a live registry.
cpp-RCP's admin.hpp instead holds a reference to shmem::Registry and
looks up endpoint/channel state through it live. NOT migrated to
c-RCP's caller-driven pattern: c-RCP's rationale for going caller-driven
was retiring rcp_registry_t entirely, which doesn't apply here --
shmem::Registry is live, current, and still the single source of truth
for channel state in this rewrite (see shmem.hpp, #153). Reasoning
recorded in admin.hpp's own header comment, not just here.

## Verification

include/rcp/admin.hpp: 128 -> 338 lines. tests/test_admin.cpp: 123 ->
200 lines. Full tree rebuilds clean (0 errors, 0 warnings) from a
from-scratch rebuild against the fully-merged rewrite/v3-from-c-rcp
base (picked up #151/#152/#153 via merge-forward first), 58/58 ctest
suites pass (100%) -- independently re-verified, not just the porting
agent's own report.

## Phase 5 status

udp (#151), l2 (#152), shmem (#153) already merged; this closes out
admin, the fourth and final "port from c-RCP" module in Phase 5's
scope. tls/capi remain as re-verify-only per ROADMAP.md Phase 17 —
confirmed clean against the new core, no content changes needed.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Matt <47545907+SoundMatt@users.noreply.github.com>

---------

Signed-off-by: Matt <47545907+SoundMatt@users.noreply.github.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
@SoundMatt SoundMatt mentioned this pull request Aug 22, 2026
Closed
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@SoundMatt