POSIX accept() semantics when the stack is polled from a separate thread - #170
POSIX accept() semantics when the stack is polled from a separate thread#170danielinux wants to merge 4 commits into
Conversation
accept() now also takes over a listener in CLOSE_WAIT, and can_read() reports any pending connection so poll()-driven servers accept it.
There was a problem hiding this comment.
🟡 Changes recommended
The ESTABLISHED/CLOSE_WAIT accept handoff currently copies the listener’s event flags into the child socket, which can trigger incorrect READABLE callbacks on the accepted fd when no RX data is queued.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates wolfIP’s TCP listener/accept behavior to better match POSIX accept() readiness semantics when the network stack is polled in one thread and accept()/poll() is performed in another. It preserves completed pre-accept connections (including queued RX data) by handing them off into a child socket, and it tightens readiness/send behavior for SYN_RCVD sockets to avoid “writable but send fails” outcomes.
Changes:
- Accept now hands off a listener in
TCP_ESTABLISHED/TCP_CLOSE_WAITinto a child socket (including RX queue and OOO segments) and restores the original fd back to a clean listener. - Listener readability (
sock_can_read) now reflects any pending connection state (not just SYN_RCVD), preventing poll/select from missing connections that have no queued payload yet. - SYN_RCVD send readiness is made consistent:
sock_can_write()returns 0 andsock_sendto()returns-WOLFIP_EAGAIN, with new/updated unit tests covering these behaviors.
File summaries
| File | Description |
|---|---|
| src/wolfip.c | Implements pre-accept ESTABLISHED/CLOSE_WAIT handoff, expands listener readability semantics, and adjusts SYN_RCVD write/send behavior. |
| src/test/unit/unit.c | Registers new/updated unit tests for the changed readiness/send and pre-accept handoff behaviors. |
| src/test/unit/unit_tests_tcp_flow.c | Updates existing pre-accept tests to assert successful handoff and adds new coverage for established-with-no-data readability and CLOSE_WAIT handoff. |
| src/test/unit/unit_tests_socket_api_arms.c | Adds focused unit tests for SYN_RCVD readability/writability and SYN_RCVD send-to returning EAGAIN. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The child no longer inherits the listener's pending-accept READABLE flag, which dispatched a read callback on an empty receive queue.
There was a problem hiding this comment.
🔵 Needs a closer look
The changes alter core TCP accept/readiness behavior and timer/event interactions in ways that are hard to fully validate without a deeper end-to-end concurrency and regression review beyond the included unit tests.
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 0 new
- Review effort level: Lite
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #170
Scan targets checked: wolfip-bugs, wolfip-src
Findings: 2
2 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Reported findings require changes before merge.
Re-arm the pure ACK the hand-off drops from the TX FIFO, and report a listener readable only in the states accept() can hand off.
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #170
Scan targets checked: wolfip-bugs, wolfip-src
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
Fenrir's latest completed scan found no issues; clearing the prior automated change request.
The problem
wolfIP has no SYN backlog — the listener socket is the pending connection, and accept() clones it out of SYN_RCVD. That only works if accept() runs inside the same wolfIP_poll() that processed the SYN. With a net thread plus an app thread doing poll()/accept(), the final ACK arrives first and the completed connection was discarded (accept() returned -1, port reverted to LISTEN).
What I'm changing:
Added tests
Three arm tests in unit_tests_socket_api_arms.c for the readiness/send changes; the three existing pre-accept tests in unit_tests_tcp_flow.c now assert the hand-off (queued bytes reach the child, stream is writable) instead of failure; plus test_tcp_listener_preaccept_established_no_data_is_readable (wildcard bind, so it covers the bound_local_ip and timer-id assertions) and test_tcp_listener_preaccept_peer_fin_hands_off_close_wait.