Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebSocketServer.run() docs and shutdown #161

Open
pikeas opened this issue Jun 5, 2021 · 3 comments
Open

WebSocketServer.run() docs and shutdown #161

pikeas opened this issue Jun 5, 2021 · 3 comments

Comments

@pikeas
Copy link

pikeas commented Jun 5, 2021

It will block until the server is accepting connections and then return a :class:WebSocketServer object

:returns: This method never returns unless cancelled.

These cannot both be true. The second remark matches the implementation, but the first remark is preferred. Currently, there doesn't seem to be a good way to cleanly shut down the server.

@pikeas
Copy link
Author

pikeas commented Jun 5, 2021

Ah, it looks like the server is returned via task.status.started(self). Perhaps the docs could be revised:

"When called by nursery.start(serve_websocket, ...) instead of nursery.start_soon, the running instance of WebSocketServer is returned."

@pikeas
Copy link
Author

pikeas commented Jun 5, 2021

With server = nursery.start(serve_websocket, ...), I believe my original comment still applies - there's no aclose() or sync close() on WebSocketServer for cleanly shutting down the server.

@belm0 belm0 changed the title serve_websocket docstring is inconsistent WebSocketServer.run() docs and shutdown Jun 5, 2021
@belm0
Copy link
Member

belm0 commented Jun 5, 2021

(I changed the title to WebSocketServer.run() but I see it also applies to serve_websocket().)

Here are the current docs with more context:

This method supports the Trio nursery start protocol: server = await nursery.start(server.run, …). It will block until the server is accepting connections and then return a :class:WebSocketServer object.

It does say that return of the WebSocketServer is done by way of start(), but could it be more clear? The reader is expected to be familiar with the Trio API and start_soon() vs. start().

Note that Trio library functions like serve_listeners() have the same API.

there's no aclose() or sync close() on WebSocketServer for cleanly shutting down the server

Trio has cancel scopes, and API implementations are able to encapsulate both synchronous and asynchronous shutdown. So the way you'd shut down the websocket server is to cancel the parent task. Note that there are two potential cancel points:

async with trio.open_nursery() as nursery:
    # cancel if it takes more than 1 second to start accepting connections
    with trio.fail_after(1):
        await nursery.start(partial(serve_websocket, ...))
    # sorry, this server will self-destruct after 5 minutes
    await trio.sleep(5 * 60)
    nursery.cancel_scope.cancel()

Everything should be cleanly shut down under such cancellations (as well as by Ctrl-C and SIGTERM)-- is there evidence otherwise?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants