Conversation
|
Could you add tests to cover this? |
|
Hi @kgaughan Tests added! |
|
Hi @kgaughan , Any chance this can be reviewed soon? |
|
How does the hand-off back to Waitress happen? This completely breaks the way that waitress handles sockets/WSGI communication. I don't think this is a good idea, nor one that the Pylons Project would like to support. This will tie up the thread for the entire time the WebSocket is alive, which is just a bad idea. Waitress was never built with this in mind. The test you added is also not an appropriate test, it just adds a new socket to the WSGI environment, not the one that is passed in, does not show the upgrade process or making sure that that this won't break anything else. |
|
Let me explain how this will work. Currently waitress does not support websocket due to not exposing the raw socket object to request handlers. We use a very popular library called simple-websocket, and we want it to be able to access the socket object and handle the communication (the upgrade process) for us. https://github.com/miguelgrinberg/simple-websocket/blob/main/src/simple_websocket/ws.py#L248 . As you can see, if we want to handle the long running communication, there has to be some thread running. Waitress supports multi threading, and it is user's responsibility to add more thread. Also, for the added extra property If you were to introduce the websocket to your project, what would be your recommended approach? |
|
It has been a while since this merge request is opened. I have also did some research how websocket should be supported through wsgi. Mainly the socket objects needs to be passed as custom extensions to the request https://stackoverflow.com/questions/75538581/does-pythons-wsgi-specification-have-anything-to-say-about-websockets Other mainstream wsgi handlers already added the support, but they are only available on Linux platform. As waitress natively supports Windows, I hope we can push this forward a bit. Thanks! |
cc572a2 to
5ca84a5
Compare
smeng9
changed the title
17cc88b to
13f6dc7
Compare
Exposing channel.socket leaves the hand-off back to waitress unspecified. The caller gets a socket the main loop still owns and that is still non-blocking, so the first recv() on it raises BlockingIOError; and when the view returns, finish() writes an HTTP response onto a connection that is no longer speaking HTTP. waitress.hijack performs the hand-off itself. It duplicates the socket, so closing it cannot pull the descriptor out from under the channel still registered in the main loop; it puts the duplicate back into blocking mode, since waitress keeps its own socket non-blocking and duplicates share that flag; and it marks the task so that no response is written and the channel closes its own descriptor once the task returns. Setting will_close also stops the main loop reading further requests from the connection, which matters when channel_request_lookahead is enabled. The thread servicing the request is occupied for as long as the application holds the connection. That cost is real and is documented rather than hidden. The name follows Rack's rack.hijack and Go's http.Hijacker, which expose the same capability in those ecosystems. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
13f6dc7 to
87a1ea4
Compare
|
Hi @digitalresistor @kgaughan — reviving this one. It's been open since May last year and I've reworked it substantially since your last look, so it's worth a fresh read rather than judging it on the earlier diff. Rebased onto current main. I have also edited the PR description at the top of the thread. |
3 participants
Rebased onto main and reworked. The first two commits are the original change; the third replaces it with an API instead of a raw socket.
This is inspired by:
Rack, which has env['rack.hijack'] (https://github.com/rack/rack/blob/main/SPEC.rdoc).
Go, which spells it http.Hijacker (https://pkg.go.dev/net/http#Hijacker).
They both return the underlying connection to the handler and allow the handler to perform WebSocket upgrades.
Note: User should increase the thread count if they want to handle long running connections.