miguelgrinberg / simple-websocket

Simple WebSocket server and client for Python.
MIT License
79 stars 17 forks source link

Racy socket operations in `close` #41

Open bcmills opened 1 week ago

bcmills commented 1 week ago

In a program that uses simple_websocket via flask_sock, we're seeing dropped or incomplete Close packets when we try to close a websocket with a closure message.

RFC 6455 § 7.1.1 says:

An endpoint SHOULD use a method that cleanly closes the TCP connection, as well as the TLS session, if applicable, discarding any trailing bytes that may have been received. … … to obtain a clean closure in C using Berkeley sockets, one would call shutdown() with SHUT_WR on the socket, call recv() until obtaining a return value of 0 indicating that the peer has also performed an orderly shutdown, and finally call close() on the socket.

Unfortunately, Base.close() does not seem to provide a way to actually do that. Instead, close() sets self.connected = False immediately: https://github.com/miguelgrinberg/simple-websocket/blob/159e030c7c23060de989cebec6d98d776c75bcbd/src/simple_websocket/ws.py#L122

which causes Base._thread() to stop reading on the next iteration: https://github.com/miguelgrinberg/simple-websocket/blob/159e030c7c23060de989cebec6d98d776c75bcbd/src/simple_websocket/ws.py#L140

and then it closes the socket immediately, even if the peer has not had the chance to perform an orderly shutdown:: https://github.com/miguelgrinberg/simple-websocket/blob/159e030c7c23060de989cebec6d98d776c75bcbd/src/simple_websocket/ws.py#L166)