miguelgrinberg / simple-websocket

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

Gevent raises ConcurrentObjectUseError after WS route sends closing frame #24

Closed emike922 closed 1 year ago

emike922 commented 1 year ago

When running a WS server with the gevent WSGIServer and sending a closing frame from within the route, gevent raises a ConcurrentObjectUseError for the socket. Adding a brief sleep before returning from the route seems to act as a workaround/mitigation.

Do you know why this occurs? Will 0.1 seconds sleep time be sufficient in all cases? Is there a more robust logic that I might employ? Is this something that might be solved from library side?

Example:

from gevent import monkey
monkey.patch_all()

from flask import Flask
from flask_sock import Sock
import gevent
from gevent.pywsgi import WSGIServer
from simple_websocket import Server
from websocket import WebSocketApp
from wsproto.frame_protocol import CloseReason

app = Flask(__name__)
sock = Sock(app)

@sock.route('/websocket', strict_slashes=False)
def websocket_route(ws: Server):
    print("Server: Socket open")
    gevent.sleep(0.1)
    ws.close(reason=CloseReason.INTERNAL_ERROR, message='close msg')
    print("Server: Socket closed")
    # Sleep to avoid gevent.exceptions.ConcurrentObjectUseError: This socket is already used by another greenlet
    gevent.sleep(0.1)

if __name__ == '__main__':
    gevent.spawn(WSGIServer(('127.0.0.1', 5000), app).serve_forever)
    ws_client = WebSocketApp(
        "ws://127.0.0.1:5000/websocket",
        on_open=lambda app: print("Client: Socket open"),
        on_close=lambda app, close_code, close_reason: print(f"Client: Socket closed, code={close_code}, reason={close_reason}")
    )
    ws_client.run_forever()

Output w/o final gevent.sleep(0.1):

Server: Socket open
Client: Socket open
Server: Socket closed
127.0.0.1 - - [2023-05-29 10:13:40] "GET /websocket HTTP/1.1" 000 - 0.101716
Traceback (most recent call last):
  File "src/gevent/greenlet.py", line 908, in gevent._gevent_cgreenlet.Greenlet.run
  File "/workspace/.venv/lib/python3.10/site-packages/gevent/baseserver.py", line 34, in _handle_and_close_when_done
    return handle(*args_tuple)
  File "/workspace/.venv/lib/python3.10/site-packages/gevent/pywsgi.py", line 1577, in handle
    handler.handle()
  File "/workspace/.venv/lib/python3.10/site-packages/gevent/pywsgi.py", line 464, in handle
    result = self.handle_one_request()
  File "/workspace/.venv/lib/python3.10/site-packages/gevent/pywsgi.py", line 660, in handle_one_request
    self.requestline = self.read_requestline()
  File "/workspace/.venv/lib/python3.10/site-packages/gevent/pywsgi.py", line 608, in read_requestline
    line = self.rfile.readline(MAX_REQUEST_LINE)
  File "/usr/lib/python3.10/socket.py", line 705, in readinto
    return self._sock.recv_into(b)
  File "/workspace/.venv/lib/python3.10/site-packages/gevent/_socketcommon.py", line 693, in recv_into
    self._wait(self._read_event)
  File "src/gevent/_hub_primitives.py", line 317, in gevent._gevent_c_hub_primitives.wait_on_socket
  File "src/gevent/_hub_primitives.py", line 322, in gevent._gevent_c_hub_primitives.wait_on_socket
  File "src/gevent/_hub_primitives.py", line 297, in gevent._gevent_c_hub_primitives._primitive_wait
gevent.exceptions.ConcurrentObjectUseError: This socket is already used by another greenlet: <bound method Waiter.switch of <gevent._gevent_c_waiter.Waiter object at 0x7f96d58fc950>>
2023-05-29T08:13:40Z <Greenlet at 0x7f96d589d260: _handle_and_close_when_done(<bound method WSGIServer.handle of <WSGIServer at , <bound method StreamServer.do_close of <WSGIServer, (<gevent._socket3.socket [closed] at 0x7f96d58c67a)> failed with ConcurrentObjectUseError

Client: Socket closed, code=1011, reason=close msg
miguelgrinberg commented 1 year ago

I do not know why this happens, but it is probably due to the Gevent WSGI server not having direct support for WebSocket. For a regular HTTP request, the server will send a response on the socket after the application's handler returns. In the case of a WebSocket connection, no response needs to be sent. I'm not sure this is the problem here, but it is a possibility. Have you tried using Gevent with the Gunicorn server? Maybe that works more reliably.

emike922 commented 1 year ago

Good call! The Gunicorn server using Gevent worker doesn't raise any such exception from what I can tell. Unfortunately, I am stuck with the regular Gevent WSGIServer for at least a while longer yet..

It appears as though by adding a simple app.config['SOCK_SERVER_OPTIONS'] = {'ping_interval': 25}, the exception is not produced either. Can you make any sense of why that would be?

Many thanks for the prompt support! :)

miguelgrinberg commented 1 year ago

@emike922 I'm looking into this. I continue to think this is an issue that occurs due to gevent's WSGI server expecting all requests end with a standard HTTP response and getting confused when the request's socket is hijacked and used for a WebSocket connection.

But while looking at their source code I've found that if the socket is closed then they exit the request silently regardless of any failures. Your sleep allowed the client time to receive the close packet that you sent, reply with its own close packet to acknowledge, and finally close the socket. Using a ping_interval changes the way socket reads are issued in the server, so this must also indirectly contribute to getting the socket closed sooner.

So basically, the goal is to figure out a way to have the socket closed before control goes back to gevent. At the same time, whatever I do to achieve this should not affect the other servers, so I have to test this well (or else make this happen conditionally only for the gevent web server, which I would prefer to avoid).

emike922 commented 1 year ago

I am actually content with using ping_interval. As long as this successfully mitigates the error without an arbitrary sleep, and it looks like it does, I will be happy to consider this a known limitation affecting the Gevent WSGIServer.

Unless this is an itch that now must be scratched, feel free to close this issue.. And thanks again!