[fpc-pascal] Websocket

[Michael Van Canneyt]

Hi,

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

Michael.

On Sun, 6 Sep 2026, Roger Olsson via fpc-pascal wrote:

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