Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens the TCP/UDP communication layer built on top of lwIP for STM32-based embedded systems. It addresses real bugs reproduced on hardware including fragmented TCP stream handling, silent packet drops, and unsafe socket lifecycle management. The changes eliminate several crash-prone and UB-inducing patterns (e.g., calling destructor directly, using nullptr PCBs without guards, direct type-punned pointer dereferences for unaligned data), and replace them with robust patterns using move semantics, null guards, stream-based reassembly, and proper resource management.
Changes:
Server,ServerSocket, andSocketare hardened with null guards, corrected move semantics, proper PCB lifecycle management, and stream-based TCP reassembly to handle fragmentationDatagramSocketfixes the move constructor self-reference bug (remote_port(remote_port)→remote_port(other.remote_port)), adds null guards, and uses pbuf copy for safe packet parsingPacketValue.hppreplaces direct pointer dereferences withmemcpyto avoid undefined behavior from type-punned and potentially unaligned reads;lwipopts.hdisables ICMP software checksumming to align with hardware offload
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
Src/ST-LIB_LOW/Communication/Server/Server.cpp |
Server lifecycle hardening: null guards, capacity enforcement, compaction loop replacing FAULT status transitions |
Src/HALAL/Services/Communication/Ethernet/LWIP/UDP/DatagramSocket.cpp |
Fixes move constructor bug, adds null guards, proper close/reconnect, pbuf-safe receive callback |
Src/HALAL/Services/Communication/Ethernet/LWIP/TCP/Socket.cpp |
Full TCP socket hardening: move semantics, stream-based reassembly, corrected callbacks, ERR_ABRT handling |
Src/HALAL/Services/Communication/Ethernet/LWIP/TCP/ServerSocket.cpp |
Server socket hardening: stream reassembly, correct accept_callback with arg fallback, idempotent close |
LWIP/Target/lwipopts.h |
Disables ICMP software checksum gen/check for hardware offload alignment |
Inc/ST-LIB_LOW/Communication/Server/Server.hpp |
Changes broadcast_order from void to bool |
Inc/HALAL/Services/Communication/Ethernet/LWIP/UDP/DatagramSocket.hpp |
In-class nullptr init for udp_control_block |
Inc/HALAL/Services/Communication/Ethernet/LWIP/TCP/Socket.hpp |
Adds rx_stream_buffer, clear_packet_queues, connect_poll_ticks, MAX_TX_QUEUE_DEPTH; tightens send_order |
Inc/HALAL/Services/Communication/Ethernet/LWIP/TCP/ServerSocket.hpp |
Adds rx_stream_buffer, clear_packet_queues, is_listening(); raises MAX_TX_QUEUE_DEPTH to 64; tightens send_order |
Inc/HALAL/Models/Packets/PacketValue.hpp |
Replaces type-punned pointer dereferences with memcpy for unaligned-safe access |
Inc/HALAL/Models/Packets/OrderProtocol.hpp |
Adds virtual destructor to the base interface class |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Robust TCP/IP Hardening
Summary
This PR hardens the ST-LIB TCP/UDP communication layer on top of lwIP, based on issues reproduced on real hardware under load, fragmentation, and reconnect scenarios.
ServerSocketlifecycle handling safer across disconnects, close/reopen sequences, and PCB reuse.DatagramSocketandpbufprocessing to avoid unstable UDP parsing paths.Impact
These changes reduce silent packet corruption, improve connection stability during stress/soak testing, and make the socket layer more reliable for production boards that depend on strict TCP session behavior.
Validation
It has been validated on a Nucleo Development Board with repeated stress, reconnect, burst, UDP round-trip, and long-duration soak tests. Those test scripts will be uploaded to the template-project
Notes
Even though it has been hardly tested on a Nucleo, this PR will remain as a draft until I validate it on a real board, because this is a critical change, and will affect Levión testing