Skip to content

refactor(server): stop branching on errno after accept/recv/send#5

Merged
vjan-nie merged 1 commit into
Univers42:mainfrom
vjan-nie:test/errno-after-io
Jul 3, 2026
Merged

refactor(server): stop branching on errno after accept/recv/send#5
vjan-nie merged 1 commit into
Univers42:mainfrom
vjan-nie:test/errno-after-io

Conversation

@vjan-nie

@vjan-nie vjan-nie commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

The 42 subject forbids using errno to drive control flow after the non-blocking I/O syscalls. acceptClient/handleClientInput/ handleClientOutput each inspected errno (EAGAIN/EWOULDBLOCK) to decide whether to log or disconnect. Removed:

  • accept() < 0 -> return (silent; no errno, no log)
  • recv() <= 0 -> disconnect only on == 0 (orderly close)
  • send() < 0 -> return
    Real socket errors (RST/ECONNRESET) are handled by the EPOLLERR|EPOLLHUP
    branch in run(); as a bonus this removes a latent false-positive
    disconnect on EINTR.

Characterization test added (RobustnessTest.AbruptDisconnectViaRST): forces a real RST via SO_LINGER and proves in black box that the nick is released (client torn down). Green before and after the refactor.

Note: which branch tears down an RST client is timing-dependent before this change (EPOLLIN+errno may win the race over EPOLLERR|HUP). After this change EPOLLERR|HUP handles it consistently, same iteration. Do not assume EPOLLERR|HUP alone covered RST prior to this commit.

Suite: 139/139.

The 42 subject forbids using errno to drive control flow after the
non-blocking I/O syscalls. acceptClient/handleClientInput/
handleClientOutput each inspected errno (EAGAIN/EWOULDBLOCK) to decide
whether to log or disconnect. Removed:
  - accept() < 0        -> return (silent; no errno, no log)
  - recv()   <= 0       -> disconnect only on == 0 (orderly close)
  - send()   < 0        -> return
Real socket errors (RST/ECONNRESET) are handled by the EPOLLERR|EPOLLHUP
branch in run(); as a bonus this removes a latent false-positive
disconnect on EINTR.

Characterization test added (RobustnessTest.AbruptDisconnectViaRST):
forces a real RST via SO_LINGER and proves in black box that the nick is
released (client torn down). Green before and after the refactor.

Note: which branch tears down an RST client is timing-dependent before
this change (EPOLLIN+errno may win the race over EPOLLERR|HUP). After
this change EPOLLERR|HUP handles it consistently, same iteration. Do not
assume EPOLLERR|HUP alone covered RST prior to this commit.

Suite: 139/139.
@vjan-nie

vjan-nie commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator Author

Closes #4


Problem

The event loop used errno (EAGAIN/EWOULDBLOCK) after the non-blocking I/O
syscalls to decide whether to log or disconnect. The 42 subject forbids
errno-driven control flow after accept/recv/send (eliminatory), and in
recv the check also mis-fired on EINTR, disconnecting a healthy client.

Change

  • acceptClient: accept() < 0return (silent; no errno, no log).
  • handleClientInput: recv() <= 0 → disconnect only on == 0 (orderly
    close); < 0return (nothing ready now).
  • handleClientOutput: send() < 0return.

Real socket errors (RST/ECONNRESET) are reaped by the existing
EPOLLERR|EPOLLHUP branch in run(). Removing the recv check also cures the
latent EINTR false-positive disconnect.

Two files only: src/Server.cpp and the new characterization test in
tests/test_robustness.cpp. include/Server.hpp unchanged. The un-polled
send() in disconnectClient is a separate correction-sheet item and is
intentionally left for a follow-up.

Verification

  • RobustnessTest.AbruptDisconnectViaRST (new): forces a real RST via
    SO_LINGER{1,0} and proves in black box that the nick is released (i.e. the
    reset client was torn down server-side), with a bounded retry (no fixed
    sleep). Green before and after the refactor — a valid characterization
    test for a refactor.
  • grep errno src/Server.cpp: no errno remains in the accept/recv/send
    branches (only socket-setup throw messages, the EINTR check on
    epoll_wait itself, and epoll_ctl logging).
  • make mandatory and full build: clean, -Wall -Wextra -Werror -std=c++98.
  • Suite: 139/139 (138 prior + 1 new).

Note for future event-loop work (please keep in the history)

Which branch reaps an RST client is timing-dependent before this change:
when EPOLLIN and EPOLLHUP arrive in the same event, the EPOLLIN path
(recv + errno) can win the race over EPOLLERR|EPOLLHUP. After this change,
EPOLLERR|EPOLLHUP handles it consistently, in the same iteration (observable
as the RST teardown reason changing from "Connection closed" to
"Connection error"). Do not assume EPOLLERR|HUP alone covered RST prior
to this commit — the earlier audit premise was corrected during review.

What I'd like you to validate

  • Builds and full suite pass in your environment.
  • AbruptDisconnectViaRST is green with src/Server.cpp reverted to
    main (characterization), and green with the refactor.
  • No errno left in the accept/recv/send branches.
  • Scope: only src/Server.cpp + tests/test_robustness.cpp;
    disconnectClient untouched (deferred).

@vjan-nie
vjan-nie merged commit 9e96ddf into Univers42:main Jul 3, 2026
2 checks passed
@vjan-nie
vjan-nie deleted the test/errno-after-io branch July 6, 2026 15:45
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.

1 participant