Skip to content

POSIX accept() semantics when the stack is polled from a separate thread - #170

Open
danielinux wants to merge 4 commits into
wolfSSL:masterfrom
danielinux:fix-rtos-late-accept
Open

POSIX accept() semantics when the stack is polled from a separate thread#170
danielinux wants to merge 4 commits into
wolfSSL:masterfrom
danielinux:fix-rtos-late-accept

Conversation

@danielinux

Copy link
Copy Markdown
Member

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:

  • wolfIP_sock_accept() (:6863) hands off a listener in ESTABLISHED or CLOSE_WAIT into a child socket instead of failing — RX queue and OOO segments included, rxbuf.data repointed, txbuf re-inited, listener timer ids dropped, bound_local_ip narrowed to the SYN's local address as in the SYN_RCVD path. The CLOSE_WAIT case also kills a 5 s poll-loop spin: a peer that connected and closed left a permanently readable descriptor whose accept() always failed.
  • wolfIP_sock_can_read() (:8266) reports a listener readable for any pending connection, not just SYN_RCVD. A client that waits for the server to speak queues no data, so the old rxbuf-only check left poll() blind and the pre-accept timer dropped a healthy connection.
  • wolfIP_sock_can_write() (:8318) → 0 and wolfIP_sock_sendto() (:7035) → -WOLFIP_EAGAIN in SYN_RCVD. Separate concern: this is the child from the normal accept path, which used to be reported writable and then fail send() with -1. API-visible change, worth reviewing on its own.

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.

accept() now also takes over a listener in CLOSE_WAIT, and can_read()
reports any pending connection so poll()-driven servers accept it.
Copilot AI lite review requested due to automatic review settings September 8, 2026 14:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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_WAIT into 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 and sock_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.

Comment thread src/wolfip.c
The child no longer inherits the listener's pending-accept READABLE flag,
which dispatched a read callback on an empty receive queue.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 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 wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@wolfSSL-Fenrir-bot
wolfSSL-Fenrir-bot dismissed their stale review September 8, 2026 16:38

Fenrir's latest completed scan found no issues; clearing the prior automated change request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants