Skip to content

fix(websocket): treat EAGAIN as "no data yet", not as a disconnect, in the socket transport - #549

Open
freitasjca wants to merge 1 commit into
HashLoad:masterfrom
freitasjca:fix/websocket-epoll-eagain
Open

fix(websocket): treat EAGAIN as "no data yet", not as a disconnect, in the socket transport#549
freitasjca wants to merge 1 commit into
HashLoad:masterfrom
freitasjca:fix/websocket-epoll-eagain

Conversation

@freitasjca

Copy link
Copy Markdown
Contributor

Summary

WebSocket receive does not work on the epoll provider. The handshake
succeeds and the client sees 101 Switching Protocols, so the connection looks
established — but nothing the client sends is ever delivered, and the server
writes a stray HTTP response onto the upgraded socket.

THorseWebSocketSocketTransport.Read treats any non-positive recv result
as a closed connection. On a non-blocking socket, recv returns -1 with
EAGAIN/EWOULDBLOCK to mean "no data available right now" — the normal
state of an idle WebSocket peer, not a disconnect.

Horse.Provider.Epoll.pas:3101 sets every accepted client socket to
O_NONBLOCK, as epoll requires. So the first Read after the upgrade marks the
connection dead and the upgrader's read loop breaks on its first iteration,
about a millisecond after the 101.

The defect

src/Horse.Provider.Socket.WebSocket.pas

Result := fprecv(FSocket, @ABuffer[0], ACount, 0);
if Result <= 0 then
begin
  Result := 0;
  FIsClosed := True;      // EAGAIN lands here
end;

EAGAIN, EWOULDBLOCK and EINTR appear nowhere in the unit. Only two
outcomes actually end a connection:

  • recv = 0 — orderly shutdown by the peer
  • recv < 0 with an errno that is not EAGAIN/EWOULDBLOCK/EINTR

Everything else means "wait and retry".

Knock-on effect

Upgrade returns as soon as the loop breaks, the route handler returns, and the
HTTP pipeline resumes — writing a full response onto a socket already handed to
WebSocket:

HTTP/1.1 200 OK
Content-Type: text/html; charset=utf-8
Content-Length: 0
Connection: keep-alive

A client mid-frame receives HTTP text where a frame should be. This contradicts
.agents/AGENTS.md: "O ciclo de vida da requisição HTTP encerra-se com o
upgrade."

It disappears with this fix, since the loop no longer exits early. A separate
guard — refusing to write to a socket after upgrade even if the handler returns
normally — may still be worth adding, but is out of scope here.

The change

Read now distinguishes the three cases and parks in select() while idle, so
a quiet peer costs no CPU:

while True do
begin
  Result := recv(...);
  if Result > 0 then Exit;
  if Result = 0 then begin FIsClosed := True; Exit; end;      // orderly close
  if not WouldBlock then                                       // real error
  begin
    Result := 0; FIsClosed := True; Exit;
  end;
  WaitReadable(WS_SOCKET_READ_TICK_MS);                        // idle: retry
end;

Two private helpers are added. WouldBlock reads fpgeterrno /
WSAGetLastError / errno; WaitReadable is a select() with a 250 ms tick
(a timeout is not a disconnect — the loop simply retries, so the tick only
bounds how often FIsClosed is re-checked).

Both carry the full four-way guard: FPC/Delphi × Windows/POSIX. uses gains
BaseUnix (FPC/POSIX), WinSock2 (FPC/Windows), and
Posix.Errno/Posix.SysSelect/Posix.SysTime (Delphi/POSIX); Delphi/Windows
already had Winapi.WinSock2.

One note on the Windows branch: Winapi.WinSock2 exposes FD_SET as a type,
not the macro-style procedure, so fd_count/fd_array are filled in directly.

No signature or behavioural change on a blocking socketrecv waits and
returns > 0, so neither helper is ever reached.

Testing

A minimal Horse WebSocket echo server (echoing from inside OnMessage) driven
by a dependency-free RFC 6455 client sending one masked "ola" text frame. The
echo can only appear if the callback runs. The route handler is instrumented
around Res.UpgradeToWebSocket, which does not return until the read loop ends
— so the elapsed time distinguishes "loop never waited" from "loop waited".

Provider Compiler This fix Result
epoll FPC 3.2.2, Linux FAILUpgrade returns in 1 ms, raw HTTP on the socket
epoll FPC 3.2.2, Linux PASS — returns after 752 ms, on peer close
IOCP Delphi 12, Win64 PASS — 769 ms
IOCP Delphi 12, Win64 PASS — 775 ms
Indy Delphi 12, Win64 PASS

Scope is epoll only. IOCP imports the same transport unit and was the
obvious candidate for the same defect, but leaves its accepted sockets blocking
— there is no ioctlsocket, FIONBIO or WSAEventSelect anywhere in that
provider, because overlapped I/O does not need non-blocking sockets. Rows 3–4
verify both that IOCP never had the defect and that this change is inert there.
Indy uses its own transport and is untouched.

Notes

The existing TestWebSocketDataExchange in
tests/src/tests/Tests.Integration.WebSocket.pas covers this path, but does not
run under either provider that would catch it — happy to follow up with a
regression test if you would like one, and to hear where you would prefer it to
live.

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