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
Graceful shutdown of the websocket connection. #448
Comments
I think this is desirable to have, as well as in-built support for checking liveness using ping/pong and deadlines. Most applications need these features. The current API mostly handles the wire format. The only thing the package does above the wire format is respond to ping and respond to close. This low-level API provides flexibility for the applications that need it (like the one I work on), but most don't need this flexibility. This package can benefit from a high-level API that wraps up all these things that applications need: shutdown, checking liveness, concurrent sends, ... |
@garyburd Can you please assign this issue to me. I would like to work on it. |
@ankur0493 Please post a proposed API and overview of the implementation for discussion. I tried to assign you to this issue, but Github will not let me do it. Please let me know what I need to do. |
(Can’t assign to non-maintainers of a repo. I think it’s fair just note
that @ankur0493 has it & follow up!)
…On Sun, Dec 9, 2018 at 7:44 PM Gary Burd ***@***.***> wrote:
@ankur0493 <https://github.com/ankur0493> Please post a proposed API and
overview of the implementation for discussion.
I tried to assign you to this issue, but Github will not let me do it.
Please let me know what I need to do.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#448 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABIcJ-3eUsrZCkdzplRzK_dqmoxrQhwks5u3dh-gaJpZM4YOjKk>
.
|
Thanks @garyburd. Will soon share the API details. |
This feature is a helper method. It's possible for applications to implement the RFC today. If an application does not initiate the closing handshake, then the application probably implements the RFC without doing anything special. Based on discussion in #487, here's my summary of the design: To avoid concurrent reads on the connection, the following two cases must be handled differently:
The common code for both cases starts with:
The remaining code for the reading goroutine case is:
The remaining code for shutdown from another goroutine is:
where c.readDone is a channel that's closed when c.readErr is changed from nil to a non-nil value. |
For someone who wants this now, I've implemented it in nhooyr.io/websocket@v1.7.0 See discussion in nhooyr/websocket#160 |
How to implement: Add field to Upgrader and Dialer:
Add field to Conn:
When a close message is sent, CloseTimeout is not zero, and maxReadDeadline is zero, then set maxReadDeadline to time.Now().Add(CloseTimeout). Set read deadline on underlying connection to the same value. Modify SetReadDeadline to honor maxReadDeadline. Use a mutex to protect against data races in the code above. Applications that set the upgrader/dialer field get graceful shutdown by sending a close message. The application’s read loop will either exit with the echoed close message or a timeout. This proposal allows any goroutine to initiate the graceful shutdown by using WriteControl to send a close message. |
It seems like to achieve a "normal" shutdown of a websocket connection, one needs to do a dance like:
The
Close
method of today closes the connection immediately, without warning or graceful shutdown. Would it be desirable to have aShutdown
method like golang's http servers, which does the dance explained above for the user?The text was updated successfully, but these errors were encountered: