I'd like to discuss fpwebsocket and fpwebsocketclient. Is this the right forum for that?

On 2026-09-05 22:58, Roger Olsson via fpc-pascal wrote:

Hi Roger,

I'd like to discuss fpwebsocket and fpwebsocketclient. Is this the right forum for that?

Yes, absolutely, it should be an appropriate forum here. :-)

Tomas
(one of FPC mailing lists moderators)

Okay, here we go:I have debugged and made a patch based on todays trunk (attached) for a fpwebsocketclient recovery/shutdown problem on Linux with OpenSSL. After a network interruption, the client could remain active or “reconnecting” while receiving no further data until the application is restarted. During reconnect/shutdown I also saw a SIGSEGV with the reader thread inside SSL_read.

Cause: fpwebsocketclient did not always stop and join its reader thread before the connection and OpenSSL objects are destroyed. It also failed to act on irClose. The result could be either a stale connection that never received data again after a network interruption, or a SIGSEGV when transport cleanup raced with SSL_read.

The best solution was to fix this inside FPC: make the message pump own the reader thread, wake a genuinely stuck socket when necessary, join the reader, and only then destroy the connection and TLS objects. I have modified my local files and done successful runtimes tests. The rest of the fpc websocket code should be unaffected.

Proposed changes
• The pump should own a named, non-self-freeing thread using FreeOnTerminate := False.
• Execute, Terminate, and destruction should be idempotent. • Shutdown should follow this order:
1. Request thread termination.
2. Allow the bounded polling loop a short opportunity to exit normally.
3. If it remains blocked, wake the socket with fpShutdown(..., SHUT_RDWR).
4. Join and free the reader thread.
5. Only afterward close and free the connection, transport, and TLS objects.

The wake operation must be raw socket shutdown only. Calling the TLS handler’s Shutdown, SSL_shutdown, or freeing OpenSSL state while another thread is inside SSL_read recreates the race.

ReadConnections should also process irClose: unregister the connection, mark the client inactive, and notify its owner, preferably after releasing the pump-list lock.

Because the reader currently holds the main connection-list lock while reading, termination needs a separate cold-path transport registry or another safe snapshot. This requires no additional per-message or hot-path locking.

For compatibility, socket interruption should only be the fallback for a reader that did not stop normally. Healthy connections should not be unconditionally shut down merely because the pump was stopped.

More in depth, the problems appear to be:

TWSThreadMessagePump.Terminate can return without proving that the worker has stopped. The timeout path clears the thread reference, allowing the connection, transport, or TLS objects to be freed while the worker may still be inside CheckIncoming/SSL_read.
ReadConnections ignores the irClose result, so a peer-closed connection can remain registered and appear active.
WaitFor alone is not sufficient: select is bounded, but after the first bytes of a frame arrive, reading the remainder of a partial WebSocket frame or TLS record can block indefinitely.

Suggested changes i detail:

Make the pump own its thread: create it suspended, set FreeOnTerminate :=
False, publish the reference, then start it.

Make Execute, Terminate, and destruction idempotent.

During termination, first request termination and allow a short bounded interval for the normal polling loop to exit.

If the reader is still running, wake it with raw fpShutdown(Socket.Handle, SHUT_RDWR), then call WaitFor and free the thread.

Do not call TSocketHandler.Shutdown, SSL_shutdown, close the descriptor, or free transport/TLS objects before the reader has joined. The OpenSSL socket handler’s shutdown path destroys SSL state and is unsafe concurrently with SSL_read.

Keep a separate cold-path registry or another safe snapshot of registered transports, because ReadConnections currently holds the main list lock while reading. The termination path must not wait for that same lock before it can wake the blocked reader.

Handle irClose by safely unregistering the connection, marking the client inactive, and notifying its owner—preferably after releasing the pump-list lock.

Preserve normal client-processing order and existing protected signatures where practical. Raw socket interruption should be the stuck-reader fallback rather than unconditional behavior, so applications that temporarily stop and restart a healthy pump retain compatibility.

Useful regression tests would cover partial-frame/TLS-record stalls, repeated Execute/Terminate, destruction during a blocked read, peer close and OnDisconnect, and multiple clients sharing one pump.

/Roger

Hi,

I maintain the websocket stuff.
Did you test whether the upgrade path still works after your changes ?

Michael.

Hi Michael,

The client-side HTTP-to-WebSocket upgrade was tested against server, including TLS connections and reconnects. FPC’s server-side TCustomWebsocketUpgrader was not tested. I only checked it statically. The only server-visible change is the addition of the non-virtual TWSTransport.InterruptRead; it is called exclusively by TWSThreadMessagePump in fpwebsocketclient. wsupgrader, fpcustwsserver, and fpwebsocketserver do not call it.

Therefore I do not expect the server upgrade path to change, but I agree that this needs a real regression test. Is there an existing upgrader test you would prefer me to extend?

Roger

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

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.

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 <

Michael,

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...

I’ve created an issue with a patch:

https://gitlab.com/freepascal.org/fpc/source/-/work_items/41900

I noticed something while looking at this:
TCertificateData.Assign in packages/fcl-net/src/sslbase.pp copies FStrData and the five TSSLData members but not FTrustedCertsDir, which opensslsockets.pp does read. I can submit a separate report for that.

Sven

I've observed a similar interruption issue in the FPC HTTP client on Windows.

After a server-timeout the client stream is corrupted. I observed it during server debugging.

If I am not mistaken, the steps-to-reproduce are:
The client sends a request -> the server takes too long to respond (e.g. because I debug the request on the server) -> client shuts down with timeout -> server finishes the request -> client sends a new request -> the client reads wrong data (maybe old data are on the socket, I don't know yet).

I have wanted to debug and fix the issue for a long time but haven't found the time to do so. I still have to check the exact steps-to-reproduce... Hopefully I'll do it soon :)

Ondrej

I checked and applied it, thank you !

Yes, please. It was added later on, if memory serves well, and the assign was probably forgotten.

Michael.

https://gitlab.com/freepascal.org/fpc/source/-/work_items/41902

Sven

After a server-timeout the client stream is corrupted.

It reproduces. A stub server that answers the first request later than the client's `IOTimeout` is enough - no debugger and no genuinely slow server needed. Same result in every run on aarch64-win64 and aarch64-linux, trunk `695611fdb3`, where `fphttpclient.pp` is byte-identical to current `main` (last changed in `99148c28`). So I could not confirm that it is Windows-specific.

With `KeepConnection := True`, a read timeout raises `EHTTPClientSocketRead` (`fphttpclient.pp:1014`, `:1379`). `DoKeepConnectionRequest` catches it at `:1672`. When it retries, `ReconnectToServer` (`:1695`) closes the socket properly. When the reconnect limit is exhausted it re-raises at `:1675` without closing anything, and the `finally` at `:1700` disconnects only on `HasConnectionClose or Terminated` - with no `Connection` header in the request, `HasConnectionClose` (`:915`, reading the *request* headers via `:750`) is False. So the caller gets an exception and the client keeps a socket whose state it does not know. The server's late response then lands on that socket and stays queued.

Measured with `KeepConnectionReconnectLimit := 0` so that the first failure reaches the caller directly; server delay 1500 ms, `IOTimeout` 300 ms, every reply carrying its own request path. In short:

```
GET /one -> EHTTPClientSocketRead (expected) client still connected: yes (this is the defect) GET /two -> "reply=1 path=/one"
```

The default limit of 1 arrives at the same place, it just needs the second failure: in a POST run the socket was still assigned after the exception had reached the caller.

Three of six Windows runs showed something extra. The body was `reply=1 path=/one` immediately followed by the *whole* second response, status line and headers included. That is `:1530`-`:1534`: whatever the header read already pulled into the buffer is written to the caller's stream in full, before `Content-Length` is looked at (`:1537`). The reads that follow do respect the remaining length (`:1541`-`:1549`), but that first write is not clipped. So on a keep-alive connection carrying more than one queued response the caller can get too much data, not merely the wrong data.

Adding a `DisconnectFromServer` to the handler at `:1672` removes the corruption - `/two` then gets `reply=2 path=/two`, on both platforms. It may belong in the `finally` instead, or `ReadResponse` may be the right place to close on error; that is a call for whoever owns the unit.

That leaves the retry, and the test grew scenarios while I looked at it, because the first answer I tried was wrong.

The obvious one - propagate `EHTTPClientSocketRead` instead of retrying it - also throws away the recovery from an abortive close (`SO_LINGER` zero, then close), which arrives as exactly the same exception. Only a *graceful* close avoids the handler altogether: `recv` returns 0, `FillBuffer` leaves through `Exit(False)` (`:1012`), `ReadResponse` returns False (`:1517`), and the retry runs through `SkipReconnect`. Scenarios 3 and 4 measure the two apart.

Which then turned up two more things, both on the stock tree:

- The re-sending has nothing to do with timeouts. When the server closes the connection gracefully as a POST arrives, the reconnect at `:1695` repeats it without asking what method it is. The stub server parsed the order twice and the caller got a 200 and no hint at all (scenario 5).

- A response that had already started is not discarded. Half a body arrives, the read then fails, and the retry appends the whole answer to what is already in the caller's stream - `"reply=2 pareply=3 path=/second"` (scenario 6). The body bytes go into the caller's stream directly (`:1534` for what the header read had already buffered, `:1386` for the rest) and the retry reuses that stream without rewinding it. I trigger the failure with a reset there rather than a stall, to keep the timing out of it; the failure class is the same one the timeout produces.

Failed checks out of fourteen, three runs each, aarch64-win64 / aarch64-linux, identical on both:

```
unchanged 6 6
+ DisconnectFromServer at :1672 3 3
+ also never retry a read error 4 4 (+1 inconclusive)
+ instead: retry only idempotent methods 2 2
```

The third line loses the three checks of the reset scenario, and scenario 6 then has nothing to look at, which is the inconclusive one.

The last line is `if not IsIdempotent(AMethod) or <limit reached> then raise` in the handler, along the lines of RFC 9110 9.2.2. It does not reach the other two, which sit on the `SkipReconnect` path and in `ReadResponse`.

What you reported is the first scenario, and the second line of the table is that scenario passing. The three that remain there are the retry findings above; the fourth line trades one of them for a behaviour change I have no standing to argue for. All of that is a separate matter from your report, and I would rather not load it onto your thread - shall I file it as its own issue?

The test is one self-contained `.pas` file, six scenarios and fourteen checks - the stub server is raw sockets on purpose, so the only fcl-web code under test is the client.

Sven

Hi Michael, Sven,

Short version: I have addressed all three issues Sven identified: waking a blocked reader on Windows, preventing a reader thread from joining itself, and avoiding interruption of healthy connections sharing the same pump.

On Windows, a blocked reader is stopped in this order:

1.

fpShutdown(SHUT_RDWR) prevents subsequent socket reads or retries. 2.

CancelIoEx(socket, nil) releases the recv operation already in progress. 3.

A persistent interruption marker unwinds the WebSocket frame parser instead of allowing another read attempt.
4.

The message pump waits for and joins the reader thread. 5.

Only then are the socket, transport, and TLS objects destroyed.

The essential safety rule is therefore preserved: no networking or OpenSSL state is freed while the reader thread may still be using it.

Only fpwebsocket.pp and fpwebsocketclient.pp are changed. The server and upgrader units remain unchanged. The actual HTTP-to-WebSocket upgrade tests pass over plain TCP and TLS.

Changes to the original code

The original TWSThreadMessagePump did not reliably control the lifetime of its reader thread.

The worker used FreeOnTerminate := True. Terminate requested termination and then polled the thread reference for approximately two seconds. If the worker remained blocked, Terminate cleared the reference without proving that the thread had stopped.

The connection, transport, socket handler, and OpenSSL objects could consequently be destroyed while the abandoned reader was still inside SSL_read. This explains the observed SIGSEGV during reconnection or shutdown.

ReadConnections also discarded the result of CheckIncoming. An irClose result therefore did not unregister the connection, clear Active, or fire OnDisconnect.

In addition, the main connection-list lock was held while reading a complete WebSocket frame. A peer sending only part of a frame could leave the reader blocked while holding that lock, preventing ordinary removal or shutdown.

The stale connection state and the OpenSSL crash were two consequences of the same incomplete reader-thread lifecycle.

What the revised patch does

The message pump now owns a named TMessageDriverThread. It creates the thread suspended with FreeOnTerminate := False, publishes the reference, and then starts it. During external termination, the pump waits for the actual thread to finish, calls WaitFor, and frees it only after it has stopped.

A healthy pump is still stopped normally. Terminate first requests termination and allows the bounded polling loop to exit without disturbing its connections. Repeated Execute and Terminate cycles therefore continue to work.

If the reader is genuinely blocked, the fallback interrupts only the transport currently inside a low-level read. A small atomic read state identifies and claims that transport. This prevents a stalled connection from causing healthy connections on the same pump to be interrupted.

No critical section has been added to normal message processing. The read path uses interlocked state changes. The separate locked transport registry is used only during registration, removal, and termination.

Why CancelIoEx is used on Windows

An isolated raw-socket test produced the following results over three rounds:

CancelIoEx(socket, nil):
Released 3 of 3 blocked fpRecv calls immediately.
fpRecv returned -1 with error 10004, WSAEINTR.

QueueUserAPC:
Released 0 of 3 blocked reads, although every APC callback executed.

CancelSynchronousIo:
Released 0 of 3 blocked reads and returned error 1168, ERROR_NOT_FOUND.

fpShutdown(SHUT_RDWR):
Accepted the call but released 0 of 3 receives already in progress.

CancelIoEx is therefore used to cancel the receive already in progress, while fpShutdown prevents the reader from entering another receive afterward.

CancelIoEx is resolved dynamically, so the patch does not introduce a new loader dependency on that entry point on older Windows systems.

A cancelled read cannot block again

The socket helper retains its interruption request until the complete read stack has unwound. It raises EWSReadInterrupted instead of allowing the frame parser or OpenSSL handler to treat the cancellation as a short read and retry.

A partially consumed WebSocket frame cannot safely be resumed, so the affected connection is removed and its owner is notified.

Peer close and callback handling

An irClose result now removes the connection from both pump registries, clears the client's active state through the existing disconnection path, and fires OnDisconnect.

Callbacks run after the main pump-list lock has been released.

Because OnDisconnect executes on the reader thread, a handler may call Terminate from that same thread. In this case, Terminate requests termination but does not attempt to join or free its own thread. A later call by the owner or destructor completes the join and cleanup.

Regression-test results

HTTP-to-WebSocket upgrade and echo: passed
Repeated Execute/Terminate, five rounds: passed
Peer close, Active=False, exactly one OnDisconnect: passed Termination with an idle healthy connection: passed TLS upgrade, echo, and peer close: passed
Partial-frame stall over plain TCP: passed; Terminate returned in approximately 125 ms
Partial-frame stall inside TLS: passed; Terminate returned in approximately 125 ms
Terminate called from OnDisconnect: returned normally Healthy and stalled clients sharing one pump: healthy client remained active and usable

The attached wsshutdowntest.pas is a command-line test for the patched WebSocket units.

On macOS, FPC targets Darwin and uses the Unix fpShutdown branch; the Windows-only CancelIoEx code is excluded. Apple documents that shutdown disables further reads, but does not explicitly guarantee that it releases a receive already in progress. A run of wsshutdowntest.pas on Apple Silicon would settle this. Scenarios 7 and 8 are the decisive blocked-read tests over plain TCP and TLS.

Please test and verify these results.

Regards,

Roger

Cool, thank you for the comprehensive research. Yes, I do not state it is Windows-specific. It's just that I work on Windows and observed it on Windows - because didn't test on other OS.

The same client compiled in Delphi with Delphi's HTTP client on Windows doesn't have this issue.

Thanks
Ondrej

Hi Roger,

I had the analysis carried out.

Summary
-------

- All three points from the first round are fixed, on both platforms. Windows is now as good as linux: Terminate on a reader blocked in a socket read returns in 110-141 ms, over plain TCP and inside TLS.
- One new hang, scenario 10: a callback parked in TThread.Synchronize while the main thread is in Terminate. Nobody services it.
- Two new observations in ReadConnections, scenarios 11 and 12, measured this time rather than argued. A connection collected for notification and destroyed by an earlier OnDisconnect callback is still used afterwards; an ordinary exception from a later client drops a notification that was already pending. Both end in a question below rather than in a verdict.
- The EndRead window I asked about last time is not hit by your patch. It took a deliberately slowed copy to see any consequence at all, so I can report what happens if it is hit, not how often it is.
- Your patch also fixes something your description does not mention: TWSMessagePump.Create never called inherited Create(aOwner).
- Unchanged, as you say: --pump-first still crashes.

Everything below was run on this machine. Measured against main 8604d19b11 - both blob hashes in your patch header match it and git apply --check passes. Two complete trees differing only in the patch, aarch64-win64 (FPC 3.3.1-20832) and aarch64-linux (WSL Ubuntu 24.04, OpenSSL 3.0.13). Each scenario runs in its own process with a 60 s external timeout, so a hang is reported rather than stalling the run.

The test is my own file back again: from "program wsshutdowntest;" onwards your copy is byte-identical to mine, so your numbers and mine come from the same code. I took your corrected header - mine still listed the old scenario numbering.

The numbers
-----------

linux without linux with win without win with passed 4 9 4 9 failed 5 2 5 2 hung 3 1 3 1

Twelve scenarios. The two failures on the patched side are the new scenarios 11 and 12, the hang there is scenario 10. Both platforms produce the same sets, not merely the same counts.

One caveat on the left-hand columns: on the unpatched tree the split between failed and hung is not stable. Scenario 8 hit the watchdog in three standalone runs on Windows but reported its checks inside the runner, so the same binary gives 5 failed / 3 hung or 4 failed / 4 hung depending on the run. The patched columns were stable across every run I made.

What is fixed:

- Windows now wakes. Scenarios 6 and 7, the partial-frame stall over plain TCP and inside TLS, hung with the first version. Terminate now returns in 110-141 ms on Windows and 101-104 ms on linux, three runs per platform, no outliers.
- No self-join: scenario 8, Terminate from the OnDisconnect callback, passes.
- No collateral damage: scenario 9, a healthy client sharing the pump with a stalled one, passes.
- irClose is acted on, over TLS as well.

The one that still hangs, and it is new
---------------------------------------

I added a tenth scenario after the review, because the question was measurable. An OnMessage callback calls TThread.Synchronize. That parks the reader thread until someone runs CheckSynchronize on the main thread. The main thread is in Terminate.

without the patch: Terminate returned after 3344 ms -> passes with the patch: never returns; the in-process watchdog reports at 45 s and the runner kills the child at 60 s

Same on both platforms. Interrupting sockets cannot help here: the reader is not in a read, it is waiting for the main thread.

To be fair about what changed: the old loop does not service CheckSynchronize either. It is merely bounded - 200 x 10 ms, then it gives up, sets FThread := Nil and leaves the thread running, which is the lifetime bug you are fixing. So the outcome moves from "a thread quietly kept running" to "the call never returns". Both are wrong; one of them is visible. TThread.WaitFor does service CheckSynchronize on both platforms (rtl/win/tthread.inc, rtl/unix/tthread.inc), but the new polling loop before it is never left. Would it be possible for the wait to service synchronization, or for the fallback to stay bounded?

Something the patch fixes that you do not mention
-------------------------------------------------

TWSMessagePump.Create(aOwner) in main never calls inherited Create(aOwner). Measured with a program that only constructs and destroys, no sockets, no threads: without the patch Owner is nil and the owner's ComponentCount stays 0; with it, both are correct.

Worth naming in the commit message, because it changes behaviour - code that passed an owner had to free the pump itself and now the owner frees it too. Freeing it before the owner stays correct, since TComponent.Destroy unregisters; a pointer kept past the owner's destruction does not.

Two more in ReadConnections, measured
-------------------------------------

Last time I listed these as things I had only read. They are scenarios now, and both reproduce on Windows and linux alike, three runs each.

Scenario 11 - a queued connection destroyed by an earlier callback. Closed connections are removed from both registries under the list lock, collected, and disconnected after the unlock. Between the first and the second of those calls the application's OnDisconnect runs, and tearing the remaining connections down from there is an ordinary thing for an application to do.

Two peers close before the pump is started, so its first pass collects both in the same pass. The test asserts that rather than assuming it: before the pump runs, both connections must report irWaiting from CheckIncoming(50,False), otherwise the round is declared undecided instead of clean. The first callback then calls Disconnect(False) on the second client, which frees its connection object, and the pump goes on to call Disconnect on that pointer.

control round, no teardown:
OnDisconnect A=1 B=1, calls after destruction=0
provocation:
OnDisconnect A=1 B=1, calls after destruction=1

So that this is an observation rather than a crash, the test connection overrides FreeInstance: the destructor and CleanupInstance run as usual, only the FreeMem is skipped, so the call lands countably in the class instead of in whatever the heap manager has put there. In an ordinary build that block is back on the free list.

Could a collected connection stay protected until its callback has run?

Scenario 12 - an exception in a later client. Both loops sit inside one try..except, so an ordinary exception from a later client jumps past the notification loop. Client A's peer closes; client B's OnMessage raises a plain Exception. Same readiness assertion as above: both must be waiting before the pump starts, or the round counts as undecided - otherwise A could be notified in one pass and B raise in another, and every check would hold without the arrangement ever existing.

control round, no failing callback:
errors=0, OnDisconnect A=1, A still tracked=False
provocation:
errors=1, OnDisconnect A=0, A still tracked=False, B still tracked=True

A is in neither state: its owner was not told, and no later pump pass can reach it, because it is in neither registry and the pending list dies with the pass. It is of course still reachable through its owning client - what is lost is the pump's own path to it. OnError fires exactly once with the callback's own message, so the exception does take the expected route, and 1.5 s of further passes produce no notification.

Could the pending notifications be drained on that path?

Both scenarios also fail on main, and it is worth saying why, because it is not the same reason: ReadConnections there discards what CheckIncoming returns, so a peer close is never acted 11 therefore never even reaches its provocation on main - the test reports that round as undecided rather than as a pass. In scenario 12 main likewise does not notify, but it does leave the connection in the list, so a later pass could still find it. Both scenarios are about code your patch introduces; neither is a regression against something that worked before.

On the locks I convinced myself there is no inversion: TThreadList.Add and Remove take and release their own lock, and InterruptConnections never touches FList. I had suspected one and was wrong.

The third one, and what it took to see it
-----------------------------------------

The EndRead window is a few instructions wide, so it is not a scenario in the numbered run but a separate --interrupt-race mode.

A correction to what I wrote last time, and to my own first reading of it: BeginRead's two checks catch a request that has already been published, which is the common case, but they do not guarantee publication before the next read - the interrupter can still be between its CAS and its InterlockedExchange. So I am not claiming to know the full set of affected readers.

What the mode stages is the case I could make deterministic: a reader that does not read again in that pass, because the frame is complete. The peer withholds half a frame so the reader parks in the payload read; Terminate waits gracefully for 100 ms before it starts interrupting, so the peer is armed from its own thread to deliver the missing five bytes 120 ms in - about 20 ms into the interrupt loop.

Against your patch the window is not hit, on either platform:

Active=False, still tracked=False, OnDisconnect=1

To find out what happens if it is hit, the same mode ran against a copy of your patch with a Sleep(50) between the CAS in InterruptRead and the InterlockedExchange that publishes the request. That is a measuring aid, not a suggestion:

Active=True, still tracked=True, OnDisconnect=0
a send afterwards: socket has been shut down / broken pipe

The connection stays registered and reports Active with a socket that has been shut down, and nobody is told. That is the consequence; how often the window is actually hit, this says nothing about.

Would it be worth having EndRead observe the request before it returns normally?

Unchanged, as you say
---------------------

--pump-first still crashes (exit 217 on both platforms). That is the missing Notification override, a pre-existing defect your patch does not claim to address.

The updated test file is attached: scenarios 10, 11 and 12 and the --interrupt-race mode are new, the rest is unchanged.

Sven

Hi Ondrej,

Understood, thanks.

The Delphi comparison fits the cause: this only shows up because the FPC client keeps the connection after the failed read, so the late response has somewhere to be queued.

The candidate is one line. In DoKeepConnectionRequest's EHTTPClientSocket handler, disconnect before deciding whether to retry or to re-raise. The retry path already reconnects; the re-raise did not, and the finally clause disconnects only on HasConnectionClose, which is False for a keep-alive request that sets no Connection header - so the caller got the exception while the client kept a socket whose state it did not know.

Measured on main 8604d19b11, fourteen checks, three runs each on aarch64-win64 and aarch64-linux, identical on both: 6 failed checks unchanged, 3 with the change. The three it clears are exactly your scenario. The three that remain are the retry findings I mentioned - a non-idempotent request repeated, and a partly received response the retry appends to - which I would rather report separately.

I will prepare a patch. The write-ups and the reproducer are ready.

Sven

Hi Sven,

The patch is now verified on both Linux and Windows using your latest test:

Main harness: 12 passed, 0 failed, 0 hung, 0 crashed --pump-first: passed
--interrupt-race: passed
Windows blocked plain/TLS termination: approximately 125 ms Windows synchronized callback termination: 16 ms

payload delivered=0 and error 10058 are expected: shutdown won the race, and the client was correctly marked inactive, unregistered, and notified exactly once.

So the previous 9/9 patch contained four real issues that are now resolved:

Scenario 10: Terminate now services CheckSynchronize while waiting. Scenario 11: disconnects are notified one at a time, so callbacks cannot invalidate queued pointers.
Scenario 12: a later callback exception cannot discard an earlier pending disconnect.
Interrupt race: one atomic state now both claims and publishes the interruption.
--pump-first: added the missing Notification override. The existing inherited Create(aOwner) correction remains included.

Hi Roger,

Sorry for this large reply.

In short:

We need to decide whether the current patch suffices for now or if a long-term overhaul of the
WebSocket implementation makes sense.

Where v3 improves upon `main`:
Based on the first 12 scenarios, `main` stands at 4 passed / 5 failed / 3 hanging, whereas v3 stands at 12 / 0 / 0 on both platforms. The issue of a hang during termination while a read operation is stuck has thus been resolved.

Where v3 performs worse than `main`:

13: Termination from a synchronized method – `main`: returns after 2–3 s (abandons the thread) – v3: hangs 14: Close-frame callback disconnects an earlier client – ​​`main`: does not report B – v3: removes an unrelated, healthy connection (stale index) 15: Reset mid-frame, Windows: – `main`: never reported – v3: busy loop, ~126,000 OnError events/s 20: Client released while its OnDisconnect is running – `main`: indeterminate (never reports) – v3: connection is released during the notification

In contrast, scenarios 17, 19, and the new scenarios 21–36 (hangs, use-after-free) are legacy issues carried over from `main`.

Long version:
-------

- Your claims hold. Every scenario from the earlier rounds that failed or hung on main passes with v3 (the two TLS scenarios measured on linux only), and --pump-first no longer crashes.
- One scenario is worse than on main, scenario 13: Terminate called from a method a callback synchronized onto the main thread. Main returns (it gives the thread up); v3 never returns.
- Four more scenarios fail on both trees, but differently with v3: a close-frame callback that disconnects an earlier client makes the pump drop a third, healthy client (14); a client freed while its OnDisconnect notification runs loses its connection under that notification (20, and 29 with a reconnect); a client that disconnects itself from its close-frame callback stops the pump from serving the others (26). Main does not reach most of these paths, because it never acts on a peer close.
- The test now has 36 scenarios. Most of the new ones fail or hang on main already: disconnecting, reconnecting or freeing a client from its own callbacks, from methods they synchronize, or while its read is stalled. They are not caused by your patch; I list them because they are in the same code and in the same test.
- On Windows, a connection reset in the middle of a frame is still never reported with v3, and OnError fires without end (15). Linux reports it.

Everything below was run on this machine, each scenario in its own process with a 45 s watchdog and a 60 s external limit, so a hang is reported instead of stalling the run.

- aarch64-win64: FPC 3.3.1, unmodified main 8604d19b11, complete snapshot build including packages. v3 built from your patch applied to the same revision. No OpenSSL on this machine, so the two TLS scenarios are skipped.
- aarch64-linux (WSL Ubuntu 24.04, OpenSSL 3.0.13): FPC 3.3.1 whose fcl-web, fcl-net and openssl sources are identical to main 8604d19b11 (the compiler itself carries unrelated local changes). v3 built from the same patch.

Before measuring, each binary was checked for what it contains: the v3 builds carry EWSReadInterrupted, the main builds do not, and the build logs show which units were loaded.

The numbers
-----------

Full runs, 36 scenarios:

linux main linux v3 win main win v3
passed 8 18 8 15 failed 19 11 17 12 skipped 0 0 2 2 hung 9 7 8 7 crashed 0 0 1 0

Per scenario (P passed, F failed, H hung, S skipped, C crashed): 4 columns: linux main, linux v3, win main, win v3
linux win
main v3 main v3
1 upgrade handshake and echo P P P P 2 repeated Execute/Terminate P P P P 3 peer close F P F P 4 Terminate while idle P P P P 5 TLS echo and peer close F P S S 6 partial frame stall, plain TCP H P H P 7 partial frame stall, TLS H P S S 8 Terminate from OnDisconnect F P F P 9 healthy client beside a stalled one H P H P 10 Terminate while a callback synchronizes P P P P 11 queued connection destroyed earlier F P F P 12 exception in a later client F P F P 13 Terminate from a synchronized method P H P H 14 close-frame callback removes a client F F C F 15 peer closes or resets mid-frame F P F F 16 OnError synchronizes a disconnect P P P P 17 OnMessage synchronizes a disconnect H H H H 18 client freed during its own callback P P P P 19 client freed, its callback synchronizes H H H H 20 client freed during its notification F F F F 21 synchronized method disconnects it H H H H 22 synchronized method reconnects it H H H H 23 OnDisconnect synchronizes a reconnect F P F P 24 OnDisconnect reconnects directly F P F P 25 client disconnects itself (message) F F F F 26 client disconnects itself (close frame) F F F F 27 stalled client freed, no Terminate H H H H 28 stalled client disconnected, no stop H H H H 29 reconnect during the notification F F F F 30 worker frees a synchronizing client P P P P 31 reconnected client freed, old callback F F F F 32 pump freed, old callback still running F F F F 33 reconnect inside its own OnDisconnect F F F F 34 pump reassigned during a callback F F F F 35 connection destructor raises F F F F 36 OnDisconnect raises during Free F F F F

Single runs - all 36 on main, 11-36 except 19 on v3 - give the same sets, with two differences on Windows main: scenario 8 hangs when run alone and fails inside the runner, as in the earlier rounds, and 14 fails alone instead of crashing. "no stop" in 27/28 means without calling Terminate first. --interrupt-race passes on all four; --pump-first passes with v3 and exits 217 on main.

Not every F is a failure. On main, 20, 23, 24 and 29 decide nothing: main never reports a peer close, so the situation they stage does not arise. On both trees, 31, 32 and 34 decide nothing either: they need the owner to act while a callback is still running, and the owner's call waits for that callback. The test says so in its output instead of counting a pass.

Worse with v3
-------------

Scenario 13. An OnMessage callback synchronizes a method onto the main thread, and that method calls Terminate - an application closing its window when a message arrives. The reader waits for the method to return; the method waits in Terminate for the reader. Main returns because its loop is bounded and then abandons the thread; v3 joins. Same on both platforms. Would it be possible for Terminate to stay bounded when it is called while the reader waits for the calling thread?

Failing on both, but differently
--------------------------------

Scenario 14. Three clients; the pump receives a close frame for B, and B's control callback disconnects the earlier client A. With v3 the pump then removes the wrong entry: C, which nobody closed, is no longer tracked by the pump, still reports Active=True, gets no OnDisconnect, and its echo is never read. Main keeps C but does not report the position be determined again after the callback, or the entry removed by pointer?

Scenario 20. The peer closes; the pump notifies the owner, whose OnDisconnect takes a while, and the owner frees the client meanwhile. With v3 Free returns at once and the connection is destroyed while the notification for it still runs (the test connection keeps its memory, so this is counted rather than a crash). 29 shows the same with a reconnect instead of Free. Could the connection stay alive until the notification has returned?

Scenario 26. A client disconnects itself from the control callback for a close frame. On both trees the connection is used after its destruction (the close reply is sent on it); with v3 the pump afterwards also stops serving the other client in the test.

Scenario 15 on Windows. With v3, an orderly close in the middle of a frame is reported after about 1.2 s; a reset is never reported, the connection stays Active and tracked, and OnError fired 911 584 times in the measuring window, 107 312 of them in the last second (the last message: "Close message already sent, cannot send more data."). What I found when I first saw this is older than your patch: TWSFramePayload.ReadData retries empty reads 100 x Sleep(1) and then raises a plain Exception ("20230316102741 TWSFramePayload.ReadData"), while the header read only checks for 0, and Windows returns -1 on every recv after a reset. On linux v3 reports the close after 112 ms and the reset after 108 ms. Is the Windows case worth a separate report?

Already on main
---------------

These fail or hang identically with and without v3. They are in the same code, so I include them, but they are not about your patch.

- Hangs: 17 and 21/22 (a method synchronized by a callback disconnects another client, or disconnects or reconnects the client whose callback waits), 19 (a client freed while its callback waits in Synchronize), 27/28 (a client whose read is stalled is freed or disconnected without calling Terminate first; the call never returns).
- Use after destruction: 25 and 26 (a client disconnects itself from its own OnMessage or close-frame callback; HandleIncoming goes on, or the close reply is sent, on the destroyed connection), 33 (OnDisconnect reconnects inside that callback; the client ends up inactive, and the new connection is not served).
- 36: OnDisconnect raises while the client is freed. Free raises the handler's exception, and neither the client nor its connection is destroyed.
- 35 checks a policy rather than a defect: a connection destructor that raises reaches the thread that disconnects. Whether that is wrong is a design question; I only report what happens.

Is disconnecting, reconnecting or freeing a client from its own callbacks, or from methods they synchronize, meant to be supported? If it is, these belong to the same area; if not, it may be worth saying so in the unit.

The test
--------

The updated test file is attached. Scenarios 1-12 are the ones from the earlier rounds with their numbering unchanged; 13-36 are new. The lifetime checks use test connections and clients whose FreeInstance keeps the memory, so a use after destruction is counted instead of crashing.

Sven

Sven, great job testing. I have a new patch solving all 36 failures. To do this cleanly, it made sense to redesign the websocket code somewhat. The goal was also to keep backwards compatibility and not break existing user code. Tested on Linux, will send the patch when it is verified also on windows.

Roger

On Sat, Sep 12, 2026 at 11:40 AM Sven Harazim via fpc-pascal <

Hi Sven, the patch now runs clean on both Linux and Windows.

It contains:

packages/fcl-web/src/websocket/fpwebsocket.pp
packages/fcl-web/src/websocket/fpwebsocketclient.pp packages/fcl-web/src/websocket/README.md
packages/fcl-web/tests/wsshutdowntest.pas

Run tests also with parameters:

./wsshutdowntest
./wsshutdowntest --pump-first
./wsshutdowntest --interrupt-race

Notice also the note in the readme that server side could benefit from a similar update.

Roger

On Sat, Sep 12, 2026 at 11:55 PM Sven Harazim via fpc-pascal <

Michael,

Should we refactor the unit tests into FPCUnit tests?

Sven

On 2026-09-15 00:53, Roger Olsson via fpc-pascal wrote:

Hi Roger,

Hi Sven, the patch now runs clean on both Linux and Windows. It contains:

.
.

First of all, I highly appreciate all your work! During your work, the created patch reached a considerable size not really suited to the mailing list well. Would it be possible to have your patch uploaded to Gitlab rather than sending it repeatedly to the mailing list? Obviously, you're welcome to discuss the current status and changes here in the list, but the size of the attachment exceeds the standard size of posts to this mailing list.

Thanks in advance

Tomas
(one of FPC mailing lists moderators)

Sven Harazim wrote

Michael, Should we refactor the unit tests into FPCUnit tests?

I think that whould be better.

Michael.

Tomas, yes of course. The patch is now uploaded to:

https://gitlab.com/freepascal.org/fpc/source/-/work_items/41913

Roger

Hi, is the patch ready to be applied or is further work needed?

Roger

Roger Olsson wrote

Hi, is the patch ready to be applied or is further work needed?

I am waiting for feedback from Sven Harazim.
I didn't have time to look at it myself yet.

Michael.

I am waiting for feedback from Sven Harazim. I didn't have time to look at it myself yet.

Give me two days.

The MR including the tests has been set up.

https://gitlab.com/freepascal.org/fpc/source/-/merge_requests/1569

Comment on 1565

https://gitlab.com/freepascal.org/fpc/source/-/merge_requests/1565#note_3875638131

Sven