Race condition in sync implemention delays closing connections

[aaugustin]

When this line:

websockets/src/websockets/sync/connection.py

Line 814 in 8c9f6fc

data = self.socket.recv(self.recv_bufsize)

runs before this line:

websockets/src/websockets/sync/client.py

Line 384 in 8c9f6fc

connection.close_socket()

closing the socket doesn't interrupt recv() because recv() isn't running in the main thread — and interruption is based on signals which are only received by the main thread. EDIT: #1721 says this is not the reason.

First discovered in #1592.

[darkhaniop]

Hi,

I ran some tests, and I think that by running the WS client itself in a secondary thread (not the one handling interrupts), this race condition can be avoided. Here's an example client code:

import threading
import time
from websockets import ConnectionClosedOK, State, WebSocketException
from websockets.sync.client import connect, ClientConnection

class MyGlobals:
    ws_client: ClientConnection | None = None

def client() -> None:
    print("client: started")
    ws_client = connect("ws://localhost:8765", close_timeout=1)

    # make the ClientConnection instance accessible from outside
    MyGlobals.ws_client = ws_client

    with ws_client:
        ws_client.send("hello")
        while True:
            try:
                msg = ws_client.recv()
                print(f"client: received '{msg}'")
            except ConnectionClosedOK as exc:
                print("client: connection closed\n", exc)
            except WebSocketException as exc:
                print("client: websocket error:\n", exc)

            if ws_client.state != State.OPEN:
                break

    print("client: done")

def main() -> None:
    print("main: started")
    client_thread = threading.Thread(target=client)
    client_thread.start()

    try:
        while True:
            time.sleep(3600)
    except KeyboardInterrupt:
        print("\nmain: received KeyboardInterrupt")

    if MyGlobals.ws_client is not None:
        print("main: closing connection")
        # MyGlobals.ws_client.socket.close()  # case 1
        # MyGlobals.ws_client.close_socket()  # case 2
        MyGlobals.ws_client.close(reason="leaving")  # case 3 (exit cleanly)

    client_thread.join()
    print("main: done")

if __name__ == "__main__":
    main()

The delays and errors were different depending on which way I chose to close the connection from the client side.

Case 1: closing with ClientConnection.socket.close()

With an echo server on the other end, when I try to interrupt the script (Ctrl-C), it hangs for several seconds, then throws EOFError (maybe from the keepalive thread? I'm not sure). Below is the example when closing with .socket.close():

main: started
client: started
client: received 'hello'
^C
main: received KeyboardInterrupt
main: closing connection
unexpected internal error
Traceback (most recent call last):
  File "/usr/local/lib/python3.12/site-packages/websockets/sync/connection.py", line 835, in recv_events
    self.protocol.receive_data(data)
  File "/usr/local/lib/python3.12/site-packages/websockets/protocol.py", line 267, in receive_data
    self.reader.feed_data(data)
  File "/usr/local/lib/python3.12/site-packages/websockets/streams.py", line 129, in feed_data
    raise EOFError("stream ended")
EOFError: stream ended
client: websocket error:
 no close frame received or sent
client: done
main: done

and the server side thread throws ConnectionClosedError:

connection handler failed
Traceback (most recent call last):
  File "/usr/local/lib/python3.12/site-packages/websockets/sync/server.py", line 609, in conn_handler
    handler(connection)
  File "/testws/example_server.py", line 9, in echo
    for message in websocket:
                   ^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/websockets/sync/connection.py", line 242, in __iter__
    yield self.recv()
          ^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/websockets/sync/connection.py", line 336, in recv
    raise self.protocol.close_exc from self.recv_exc
websockets.exceptions.ConnectionClosedError: no close frame received or sent

Case 2: closing with ClientConnection.close_socket()

In the above script, if I use ws_client.close_socket() instead, it exits immediately. So, this is a better option. However, connection loops exit with exceptions.

Client output:

main: started
client: started
client: received 'hello'
^C
main: received KeyboardInterrupt
main: closing connection
client: websocket error:
 no close frame received or sent
client: done
main: done

The server-side thread exits with the same error as in Case 1.

Case 3: closing with ClientConnection.close()

This is the cleanest one: the client closes the connection cleanly and exits without delays. Client output:

main: started
client: started
client: received 'hello'
^C
main: received KeyboardInterrupt
main: closing connection
client: connection closed
 sent 1000 (OK) leaving; then received 1000 (OK) leaving
client: done
main: done

And no errors on the server side.

[aaugustin]

This race condition affects the opening handshake only. Your tests are after the opening handshake so they don't reproduce it.

Also, it's a race condition, meaning that it doesn't happen every time. It depends which one of the two threads (the user thread that called connect and the background thread that manages the TCP connection) resumes running first.

[darkhaniop]

Thanks for clarifying the issue. Indeed, I was testing ways of gracefully closing the WebSockets sync client running in a secondary thread. I noticed the delay (and exceptions) when closing the connection and came here assuming this issue discusses the same situation. Turns out that's not the case.

In my tests, I ran the client in a secondary thread, and did not find an example of terminating such a connection after calling ClientConnection.recv() on the quickstart and FAQ pages.

In case someone stumbles into my example above. I looked further into the docs and saw that ClientConnection.recv() accepts an optional timeout parameter, which gives a way of properly exiting the loop:

# ...
def client(exit_event: threading.Event) -> None:  # pass exit_event from the calling (main) thread
    # ...
    with ws_client:
        ws_client.send("hello")
        while not exit_event.is_set():  # the calling thread can end this loop with exit_event
            try:
                try:
                    a = ws_client.recv(timeout=0.5)  # use timeout
                except TimeoutError:
                    continue
                print(f"client: received '{a}'")
            except ...

I am still curious about this race condition during the handshake, so I will look into the source code to try to understand it.

[darkhaniop]

Unfortunately, I couldn't reproduce this "delay." Before I explain what I did, let me confirm if I understood the bug correctly. When we call connect() (from websockets.sync.client):

• a TCP connection with the server gets established;
• the second thread is initiated when create_connection() is called at websockets/sync/client.py#L364;
• and it reaches the try-except block in websockets/sync/client.py#L377-L386;
• when the handshake fails, connection.close_socket() is called on line 384;
• and this call could get stuck if at the same time the other thread is already awaiting for data at websockets/sync/connection.py#L814

Is this correct? In any case, are there additional details about this bug? E.g., under what kind of conditions it was observed, for how long the script gets stuck (or does it get blocked indefinitely), and does it happen on specific types of connections (TCP, UDS).

My results

Somehow, in my tests blocking/delay does not happen. To trigger the described conditions, in connection.py I put a block right before the self.socket.recv() call:

# inside def recv_events()
my_event.wait()  # threading.Event, blocks the thread to ensure socket.recv() is called at the desired moment
print(f"{time.monotonic()} socket.recv() ...")
data = self.socket.recv(self.recv_bufsize)
print(f"{time.monotonic()} socket.recv() done")
# ...

And in client.py, I forced the handshake failure and the socket.recv() call (in another thread) by inserting these lines:

    try:
        raise Exception  # go straight to the exception
        connection.handshake(
            additional_headers,
            user_agent_header,
            deadline.timeout(),
        )
    except Exception:
        my_event.set()  # unblock the socket thread
        time.sleep(1)  # just to be sure the other thread calls socket.recv() before the next line
        print(f"{time.monotonic()} connection.close_socket() ...")
        connection.close_socket()
        print(f"{time.monotonic()} connection.close_socket() done")
        connection.recv_events_thread.join()
        print(f"{time.monotonic()} recv_events_thread joined")
        raise

And this is the execution order I get:

424723.400321678 socket.recv() ...
424724.401619214 connection.close_socket() ...
424724.402313408 connection.close_socket() done
424724.402342283 socket.recv() done
424724.402656934 recv_events_thread joined

There is a one-second delay here, because of the inserted time.sleep(1), but I did not see additional delays.

By placing my_event.set() inside close_socket() to see if the race condition can be triggered if some of the lines in close_socket() get executed before the socket.recv() call in another thread, but could trigger a delayed shutdown. In my tests, the blocked socket.recv() always returned right after self.socket.shutdown(socket.SHUT_RDWR) in the client thread.

My env:

• Debian 12;
• Python 3.11 and 3.12;
• I tried with a normal WS server, and a server process that only accepts the connections but does not send any data.

[aaugustin]

I believe I understand why the issue is happening.

First, for context, in WebSocket, the proper way for a client to close a connection is to half-close it then wait for the server to close it. The library is designed to do this, and that's why it calls recv(), expecting to receive EOF.

The problem discussed here occurs when a client attempts to connect and the server sends a non-101 response with Connection: keep-alive and does not close the connection.

In that case, websockets attempts to close the socket immediately but, as diagnosed by @scttbnsn, this does not interrupt recv() reliably on macOS.

Thanks a lot for clearing my misconceptions about the cause!

---

The fix proposed in #1721 feels awkward: I don't like the "make a note here then apply there" pattern, as it's difficult to reason about its correctness; more importantly, it's a symptom that we may be fixing at the wrong layer.

I've also been wondering if the same problem could happen elsewhere, and it can happen on the server during the opening handshake:

sync/server.py:751-758 — when the incoming bytes don't even parse as HTTP (ServerProtocol.parse(), server.py:598-608), send_eof() fires immediately, self.request stays None, so ServerConnection.handshake() skips its send_context()-guarded block entirely (sync/server.py:146) and raises directly. The handler thread's except TimeoutError/except Exception branches then call connection.close_socket() + connection.recv_events_thread.join() immediately, with no wait — structurally identical to client.py:387.

However, it doesn't seem possible to trigger the problem past the opening handshake — that would be during the closing handshake:

Why the opening-handshake case was vulnerable: client.py:387 calls close_socket() completely unconditionally, with zero relationship to any timeout — recv_events() might have just started a fresh, full-length close_timeout-bound recv() when the other thread barges in and tries to force it shut. There's no synchronization between "when the closer decides to give up" and "how much time is left on the blocked recv()."

Why the closing handshake doesn't have that problem: every close_socket() call reached during/after OPEN → CLOSING is gated behind send_context()'s wait_for_close logic (connection.py:1001-1023), and that logic and recv_events()'s own socket timeout (connection.py:824-825) both read .timeout() off the same self.close_deadline object — a single Deadline created once (connection.py:970-971 or :992-993) and shared. Deadline.timeout() (utils.py:42) just computes self.deadline - time.monotonic() against a fixed target instant, so no matter which thread asks, and no matter when, both are counting down to the identical wall-clock moment. That means recv_events()'s own blocking recv() will time out on its own, via the kernel's SO_RCVTIMEO, at essentially the same instant any other thread's join(timeout) would also expire — there's no window for one thread to need an external wakeup of an already-blocked recv(), which is specifically the thing that's unreliable on macOS. Any other thread (the closer, the keepalive thread) generally finds recv_events_thread already dead by the time its own wait elapses, or arrives at essentially the same moment recv_events() is timing out anyway.

---

A slightly more elegant fix might be:

                    if self.protocol.close_expected():

                        # ADDED THIS
                        # If we fail during the opening handshake, then we won't read more data anyway,
                        # and we never want to wait for the other side to close the connection as it could
                        # keep it open, e.g. an server sending an HTTP response with keep-alive enabled.
                        if self.protocol.state is CONNECTING:
                            self.close_deadline = Deadline(0)

                        if self.close_deadline is None:
                            self.close_deadline = Deadline(self.close_timeout)

This feels like a fix at the right level, because it cause recv() to return immediately in circumstances where we have good reasons to what that.

EDIT: actually it's not ideal:

side-effect I noticed while tracing this: it routes through the generic error handler, which sets recv_exc to a synthetic TimeoutError.

Maybe I'm slowly converging on the approach in the current PR...

---

I'll continue tomorrow :-)

[aaugustin]

Scott - thank you very much for your help in understanding the issue - probably I'll take it from there and write a test the way I want to write it :-)

[scttbnsn]

No worries at all, glad it helped. Makes sense to take it from here and write the test the way you want it.

One thing that might be useful for the test: the delay isn't limited to the unparseable-request case — the normal process_request rejection (server returns a non-101, e.g. 403) has it too. I measured the server's reader thread riding out close_timeout (~302ms at close_timeout=300ms, same on main and this branch) when the client keeps the TCP connection open. Same "delays closing connections" symptom on the server side, independent of this PR.

Thanks for taking the time to work through it.

[aaugustin]

the normal process_request rejection (server returns a non-101, e.g. 403) has it too

Yes, this must be the most likely scenario triggering this problem.

[aaugustin]

The script is minimal_shutdown_repro.py. Here's the summary across all the runs so far, with RECV_TIMEOUT=1s:

┌─────────────────────────────────────────────────────────┬─────────────────────────────────────────────────────────┐
│                        Scenario                         │                         Result                          │
├─────────────────────────────────────────────────────────┼─────────────────────────────────────────────────────────┤
│ No half-close, shutdown()+close() from another thread   │ flaky (17–42% hang, varies run to run)                  │
├─────────────────────────────────────────────────────────┼─────────────────────────────────────────────────────────┤
│ Half-closed, then close() alone from another thread     │ 100% hang, every single trial, every run                │
├─────────────────────────────────────────────────────────┼─────────────────────────────────────────────────────────┤
│ Half-closed, then shutdown()+close() from another       │ flaky but clearly worse than no half-close (57–58%      │
│ thread                                                  │ hang)                                                   │
├─────────────────────────────────────────────────────────┼─────────────────────────────────────────────────────────┤
│ Half-closed, but the peer closes its end instead        │ 0% hang, every single trial, every run — always instant │
└─────────────────────────────────────────────────────────┴─────────────────────────────────────────────────────────┘

The two bolded rows are the proof: after shutdown(SHUT_WR), plain close() from another thread never wakes up the blocked recv() — not once across 50+ trials — while the peer actually closing its end always does, instantly, every time. That's the core claim demonstrated with a deterministic result, no flakiness needed. The shutdown()+close() combination (what real code typically does, hedging its bets) is in between and genuinely racy — which matches the flakiness we saw reproducing the actual websockets bug — but even at its best it's meaningfully worse once the socket has been half-closed first.

(all this on macOS)

[aaugustin]

Eventually I landed on the exact same fix as #1721. It's expressed a bit differently, but the behavior is the same.

This still needs a test. The proposal in #1721 "reproduces" in the connection layer by copy-pasting bits of the protocol layer, so we aren't testing the actual implementation; I'll see if I can do better.

The purpose of the test is mostly to exercise the new branch, and demonstrate that it's actually taken in the scenario discussed here.

Since the problem was never reproducible in a deterministic way, if we reintroduced the issue somehow, the test would become flaky instead of failing; I'm relying more on the comment than the test.

[aaugustin]

Hrm, after writing a test expected to reproduce the issue, the fix doesn't appear to work as intended :-(

Indeed, for a client, self.process_event(event) merely sets an event to notify the thread running handshake() that a response has been received. self.protocol.close_expected() still returns False until that other thread executes and find that the handshake failed. So we don't break out of the loop; we run recv() again; and we can have race conditions.

EDIT: I was just confused again :-(

[aaugustin]

We're really hitting a structural hard point with the design of our layers (Sans-I/O + I/O), which is really predicated on running recv() constantly an producing events, then interrupting it when we want to close the connection, in a layer that doesn't have protocol-specific logic.