Harden TCP/UDP socket loops and use RTHS-friendly network defaults#6
Open
joecga4 wants to merge 1 commit into
Open
Harden TCP/UDP socket loops and use RTHS-friendly network defaults#6joecga4 wants to merge 1 commit into
joecga4 wants to merge 1 commit into
Conversation
Robustness (bug fixes): - The 9 recv/send loops in TCP_Socket never checked the return value. If the peer closes the connection recv() returns 0 and the loop spins forever; on error (-1) the pointer moves backwards and the byte count grows (memory corruption). All loops now treat <= 0 as an error and return -1, retry on EINTR, and the send paths restore the caller's byte-swapped data before bailing out. - Same treatment for the 8 recvfrom/sendto chunk loops in UDP_Socket, which additionally advance by the actual byte count instead of the requested chunk size. - TCP_SocketSSL treated SSL_read() == 0 (clean shutdown) as success and looped; the checks are now <= 0. Latency/reliability defaults (hybrid simulation traffic is small request/response messages exchanged every integration step): - TCP_Socket/TCP_SocketSSL now default to noDelay = 1. With Nagle + delayed-ACK enabled a distributed hybrid test can stall ~40 ms per step. Callers that pass noDelay explicitly are unaffected. - MAX_UDP_DATAGRAM 9126 -> 1432 so a logical datagram fits in one MTU; with 9126 every datagram is IP-fragmented and one lost fragment drops the whole message. Both ends of a UDP channel must use the same value (Socket.h, Socket.cpp and socket.c are kept in sync). - UDP_Socket requests 512 KB SO_RCVBUF/SO_SNDBUF in its three constructors so per-step datagram bursts are not dropped. These are ports of fixes validated in OpenFresco's vendored copies of the same files (distributed 3-process hybrid simulations over TCP and UDP, bit-identical outputs; measured max DAQ round-trip 1224 -> 82 us after the UDP changes). 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.
Port of the socket fixes already validated in OpenFresco's vendored copies of these files, applied to
SRC/actor/channel.Robustness (bug fixes)
TCP_Socketnever check the return value. If the peer closes,recv()returns 0 and the loop spins forever; on error (−1)nleft -= -1grows the count and the data pointer moves backwards (memory corruption). All loops now treat<= 0as an error and return −1, retry onEINTR, and the send paths restore the caller's byte-swapped data before bailing out.recvfrom/sendtochunk loops inUDP_Socket, which now also advance by the actual byte count instead of the requested chunk size.TCP_SocketSSLtreatedSSL_read() == 0(clean shutdown) as success; checks are now<= 0.Latency/reliability defaults (hybrid-simulation traffic is small request/response messages every integration step)
TCP_Socket/TCP_SocketSSLdefault tonoDelay = 1. With Nagle + delayed-ACK a distributed hybrid test can stall ~40 ms per step. Callers passingnoDelayexplicitly are unaffected.MAX_UDP_DATAGRAM9126 → 1432 so one logical datagram fits in one MTU; at 9126 every datagram is IP-fragmented and one lost fragment drops the whole message. Both ends of a UDP channel must use the same value, soSocket.h,Socket.cppandsocket.care kept in sync (OpenFresco ≥ 2026-07 already uses 1432).UDP_Socketrequests 512 KBSO_RCVBUF/SO_SNDBUFin its three constructors so per-step datagram bursts are not dropped.Validation: full OpenSees suites green (pytest 7/7, verification 18/18 byte-identical, MP/SP/PyMP MPI runs); OneBayFrame distributed 3-process hybrid simulations against OpenFresco, including
genericClient↔startSimAppElemServerruns over both TCP and UDP (these exercise exactly this code), 1791 steps with outputs bit-identical across transports. In OpenFresco the same UDP changes measured max DAQ round-trip 1224 → 82 µs.🤖 Generated with Claude Code