Roger,
Do you think you can fix the issues raised by Sven ?
The windows stuff, same-thread join, InterruptConnections issue.
Sven, if you can file an issue for the TWebSocketServer.CertificateData
issue (patch would be nice, but not required) so that can be scratched off
the list...
Michael.
On Mon, 7 Sep 2026, Sven Harazim via fpc-pascal wrote:
Hi Roger, hi Michael,
I built two complete trees from trunk 695611fdb3 differing only by this
patch and ran them against each other on aarch64-linux (Ubuntu 24.04,
OpenSSL 3.0.13) and aarch64-win64. The patch applies cleanly to that
revision; a full `make all install` with it succeeds on linux. Everything
below is from runs on those two machines, not from reading the code.
To answer Michael's question first: the upgrade path is unaffected. The
test drives FPC's own TWebSocketServer against the client, so the real HTTP
handshake runs. It passes with and without the patch, over plain TCP and
over TLS. I did not find any server-side caller of the new
`TWSTransport.InterruptRead` either — the only call site is
`TWSThreadMessagePump`.
What the patch fixes, measured. Without it, a peer-closed client stays
`Active=True` and never fires `OnDisconnect`, over both transports and on
both platforms — the stale connection from the original report. With it,
both are correct. And where a reader is genuinely stuck mid-frame, on linux
`Terminate` now returns in ~300 ms and the teardown is clean, against ~2 s
plus a hanging client destroy without it.
Where it does not work: Windows. The wake is a raw
`fpShutdown(SHUT_RDWR)`, and on win64 that does not release a `recv` that is
already blocked. Isolated raw-socket test, three rounds, no websocket code
involved:
linux : released after 0-2 ms, recv returns 0
win64 : NOT released within 3000 ms, 3 of 3 rounds
(fpShutdown itself returns 0)
Because `Terminate` then joins unconditionally, it blocks for as long as the
read does. The old code gave up after ~2 s — though the old destructor could
already block on the list lock the reader holds, so this is not a clean
regression, it moves where the hang happens.
Same test, immediately afterwards on the same socket:
win64 : closesocket DID release it after 0 ms (recv=-1, WSAENOTSOCK)
So a wake that works on Windows exists, but it is the one the patch
deliberately avoids — closing the descriptor is exactly what races with TLS
teardown. That looks like a real trade-off rather than an oversight, so I am
not proposing a fix; you two know the constraints. I am happy to measure any
candidate on this machine, including `SO_RCVTIMEO` on the client socket,
which the OpenSSL handler already anticipates but which interacts with the
100-retry loop in `ReadData`.
Two more things I ran into, both reproducible on linux with the patch:
`InterruptConnections` shuts down every registered transport, not just the
stuck one. With a healthy echo client and a stalled client on the same pump,
right after `Terminate` the healthy client still reports `Active=True` with
zero `OnDisconnect` calls, but sending on it raises
`EWebSocket: Write operation returned error: (104) Connection reset by peer`.
The owner is never told its connection is gone.
Calling `Terminate` from the `OnDisconnect` handler makes the reader thread
join itself, because `ReadConnections` invokes the callback on that thread.
On win64 `Terminate` is entered and never returns; on linux/glibc it raises
`EThreadDestroyCalled`. Reacting to a disconnect by stopping the pump seems
a fairly ordinary thing for an application to do, and the patch is what
makes this path reachable — before it, the pump discarded the `irClose`
result and never delivered the notification.
Unrelated, noticed while writing the TLS tests:
`TWebSocketServer.CertificateData` is declared but never instantiated, so
`CreateSSLSocketHandler` dereferences nil as soon as a TLS handler is
requested. I worked around it in the test with `OnGetSocketHandler`. Happy
to file that separately if it is not already known.
One last note: there are currently no websocket tests in FPC at all —
`packages/fcl-web/tests/` covers HTTP, HTTP/2, JWT, cookies and routes only.
So there is no upgrader test to extend, Roger.
The two programs I used are attached (18 KB, sources plus a README with the
build lines and the numbers above). They are standalone: point `-Fu` at the
websocket sources of whichever tree you want to test, so the same binary can
be built against a patched and an unpatched one. Each scenario runs in its
own process with a timeout, so a hang is reported instead of stalling the
run. If a harness in `fcl-web/tests` would be useful, I am happy to shape
them into that form — right now they are deliberately standalone so they can
be pointed at two trees at once.
Sven
_______________________________________________
fpc-pascal maillist - [email protected]
https://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal
