Michael, Yes, I'm on it. Will send new patch in a day or two.
Roger On Mon, Sep 7, 2026 at 10:39 PM Michael Van Canneyt via fpc-pascal < [email protected]> wrote: > > 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 >
_______________________________________________ fpc-pascal maillist - [email protected] https://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal
