Repository navigation
fix(network): prevent CoT TLS read-buffer stalls - #2296
Merged
Merged
Conversation
CotProtocol kept the 1 KB packet that protocol detection hands over and read into it for the life of the connection. Over ssl:// a TLS record is indivisible and decrypts to up to 16 KB, so the first record carrying more than 1 KB of CoT made SSLEngine.unwrap return BUFFER_OVERFLOW with nothing consumed. readBuffer did not handle BUFFER_OVERFLOW: the record stayed in encryptedIn, the buffer filled, socket reads returned 0 and the receive queue stopped draining while the reader looped on the buffered byte count. Small records decode, so the feed ran for minutes and then stalled for good on every connection. CotProtocol now reads into its own buffer sized from serverReadBufferSize, never below 32 KB and capped at 1 MB. readBuffer hands decoded data back on BUFFER_OVERFLOW so the caller can drain, and closes the endpoint when an empty buffer cannot hold a record instead of hanging. Also: - the reader stops looping on a read that decodes no payload and waits on the selector instead - the endpoint is closed when the inbound buffer is full on BUFFER_UNDERFLOW rather than spinning - delegated engine tasks requested after the handshake are run, since the finished handshake manager no longer runs them - the pooled reader thread is no longer renamed with an extra prefix on every run Observed on central's IHMC CoT listener: receive queue pinned at ~205 KB minutes after each connect, with the CoT reader burning CPU. NO-ISSUE Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 3332f28)
Integrate PR #2290 with current development while retaining the original contributor commit and its authorship in history. Preserve current TLS handshake writes and CoT streaming behavior. Refs: MSG-415
Exercise real TLS 1.2 and TLS 1.3 records, buffered overflow retries, fragmented input, delegated tasks, full-buffer underflow and CoT detection handoff. Retain existing TLS handshake regressions. Refs: MSG-415
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Run the reader under a scoped daemon mock so the handoff assertion remains deterministic after other protocol tests start the server. Refs: MSG-415
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
2 participants
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.
CoT reused the 1 KiB protocol-detection buffer for the connection lifetime. A larger TLS record could return BUFFER_OVERFLOW without consuming bytes, stalling the feed while the reader spun. Allocate a dedicated CoT read buffer (32 KiB minimum, 1 MiB cap), handle SSL overflow/full underflow explicitly, stop empty-payload draining, run delegated tasks and restore pooled thread names.
Integrates the contributor's original commit from #2290 with current development. The original commit and authorship remain in history. This PR supersedes #2290; do not merge both. Existing MSG-331 handshake-write handling, timeout protection, buffered-read detection and current CoT streaming/publishing/outbound behavior are retained.
Validation at head 6da368d:
Keep draft until combined regression validation is green. Jenkins: https://jenkins.mapsmessaging.io/job/mapsmessaging-server-junit/241/
Known pre-existing limitation: protocol detection still uses a 1 KiB buffer before CotProtocol construction; this change addresses established CoT reads.
Refs: MSG-415
https://mapsmessaging.atlassian.net/browse/MSG-415