Dispatch rtc callbacks off the rtc threads; TS-parity full-node test scale#94
Merged
ptesavol merged 3 commits intoJul 18, 2026
Merged
Conversation
Collaborator
Author
|
streamr-dht lint is green after a full-tree rebuild — the earlier red was the documented stale-modmap false positive (test TU modmaps regenerate only when their targets build). No source change needed. |
Filtered-out trace/debug calls still marshalled their metadata to JSON and scanned the environment per call (FollyLoggerImpl) — 20-90 ms per network message in Debug builds, dominating the connection delivery chains under the full-node tests. The constructor snapshots the minimum enabled level across the logger's own level and every LOG_LEVEL_<Category> override (env vars cannot meaningfully change mid-process; messages that pass the bound are still re-checked per category as before), and log() returns before any marshalling when the message level is below it. Adopted from the accept-path investigation (session branch claude/nostalgic-bell-34f492, 2026-07-13). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
libdatachannel funnels ALL websocket TCP I/O through ONE global poll thread; websocket upgrades and message delivery ran inline there, and data channels share the fixed-size rtc worker pool with ICE/DTLS processing. Running the emit chains (handshake, RPC parse/dispatch) inline starved every other connection in the process: measured the poll thread >95% busy in Data emit chains while 12 concurrent connectivity checks timed out at their 1 s limit — the accept-path degradation that kept the full-node tests at 8 nodes. - WebsocketConnection/WebrtcConnection own a per-connection SharedSerialExecutor; every rtc callback is a thin wrapper that only enqueues the real body. Per-connection FIFO preserves the emitter contract (Connected before Data, Data in order, Disconnected last) — the Simulator's per-association idiom. - Teardown rules the offload requires: drainDispatchQueue() barrier; WebsocketServer::stop() quiesces the accept thread, swaps out the half-ready map and drains each connection (their listeners capture the server raw); handleIncomingClient gates on mStopped; the half-ready Connected handler uses find() (stop() may have swapped the map); WebsocketServerConnector's server-Connected handler drops late connections once aborted. - WebsocketServerConnection::getRemoteAddress(): libdatachannel returns host:service — strip at the LAST colon. The unstripped value made every connectivity probe dial ws://ip:port:port, so node descriptors lacked websocket info and links silently fell back to WebRTC. - Full-node tests restored to TS-parity node counts (websocket 12, webrtc 22). Adopted from the accept-path investigation (session branch claude/nostalgic-bell-34f492, 2026-07-13) and revalidated on top of the teardown fixes: websocket 12-node run now 3 s (was: failed SetUp beyond 8 nodes); on a CI-sized 4-thread pool 10/12 runs green with zero hangs, the two failures being the known pre-existing Handshaker-interleave UAF (follow-up filed). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ptesavol
force-pushed
the
claude/rtc-dispatch-offload
branch
from
July 17, 2026 09:29
e710bf8 to
3e61c5a
Compare
…map erases 1-in-10 runs of the 22-node WebrtcFullNodeNetworkTest hung AFTER gtest printed PASSED: exit-time static destruction blocked in CPUThreadPoolExecutor::~CPUThreadPoolExecutor -> joinKeepAliveOnce on a KeepAlive held by a leaked WebrtcConnection's dispatch executor. A WebrtcConnection is immortal until doClose() runs (its rtc callbacks and signalling listeners capture a strong self), and two attempt-map paths abandoned one without ever closing it: - pendingConnection Disconnected erased the attempt (the last owner stop() could still destroy) without closing the connection; for an answerer that never received a handshake request, IncomingHandshaker has no pending->connection close link yet, so nobody else closed it either. Triggers observed in lifecycle-registry logs: the pending connection's 15 s connect timeout and endpoint close at teardown. - a stale Disconnected from an older connection to the same nodeId erased a freshly inserted attempt (insert and erase within the same millisecond in the logs), orphaning the new pair at birth. All three erase handlers now erase only the pair that fired (raw-pointer identity comparison, never dereferenced), and the pendDisconnected handler closes the paired connection outside mMutex. Verified with repeated isolated runs under CPU load: baseline 4/15 exit-hangs (every hung process held exactly one never-closed answerer connection; clean runs held zero) -> 0/15 with the fix, plus 0/5 on the de-instrumented build; streamr-dht unit 181/181 and integration 43/43. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Stacked on #92 (retarget to main after it merges). Adopts and revalidates the accept-path investigation from session branch
claude/nostalgic-bell-34f492(2026-07-13, uncommitted at the time), which diagnosed why the full-node tests were capped at 8 nodes: libdatachannel runs all websocket I/O through one global poll thread, and the emit chains ran inline there — the poll thread measured >95% busy while concurrent connectivity checks timed out at 1 s.Changes
Logger:log()early-outs below the cheapest enabled level before marshalling metadata to JSON (filtered-out trace/debug cost 20–90 ms per delivered message in Debug builds).WebsocketConnection/WebrtcConnection: every rtc callback is now a thin wrapper that enqueues the real body onto a per-connectionSharedSerialExecutor(per-connection FIFO keeps the emitter contract: Connected → Data in order → Disconnected). Includes the teardown rules the offload requires (drainDispatchQueue()barrier,WebsocketServer::stop()half-ready-map swap + per-connection drain, late-connection drop after connector abort).WebsocketServerConnection::getRemoteAddress(): strip the:servicesuffix libdatachannel appends — the unstripped value made every connectivity probe dialws://ip:port:port, silently downgrading node-to-node links to WebRTC.Validation (on top of #92's teardown fixes)
Known open item
One of 11 normal-pool WebRTC runs hung after the test passed, in exit-time static destruction:
~CPUThreadPoolExecutor → joinKeepAlivewaiting on an unreleased executor KeepAlive (thread sample attached to the investigation notes).leaks --atExitshows zero unreachable blocks, pointing at a still-reachable abandoned connection (rtc callbacks capturedselfshared_ptrs before this change too) now pinning a dispatch-executor KeepAlive. Filed as a follow-up; pre-existing object-lifetime issue whose symptom this patch changes from silent memory retention to a rare exit stall.🤖 Generated with Claude Code