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

Better document websocket::stream::async_close and teardown relationship #2730

Open
ecorm opened this issue Aug 28, 2023 · 5 comments
Open

Comments

@ecorm
Copy link

ecorm commented Aug 28, 2023

Version of Beast: Boost 1.83.0

The documentation for websocket::stream::async_close does not specify what happens to the underlying TCP socket in both success and error paths (same goes for websocket::stream::close).

I'm guessing the teardown customization point has something to do with this, but there is no mention of it in the async_close documentation.

In particular, I need to know if the underlying TCP socket is closed when websocket::stream::async_close completes either successfully or with an error.

I know that I can do if (websocket_.next_layer().is_closed()) in the completion handler to find out at runtime, but I'd still like to know the behavior.

ADDENDUM: websocket::stream::async_close also does not document whether is_open() == false is a postcondition whether or not the operation succeeds. I would expect is_open() == false to always be a postcondition whether or not it succeeds.

@ecorm
Copy link
Author

ecorm commented Aug 29, 2023

I have a question related to this. When websocket::stream::async_close fails, will read operations receive an error (so that the application knows to end)?

@klemens-morgenstern
Copy link
Collaborator

What happens to the tcp socket depends on the other side. I think !is_open() is not a post-condition.

It sends the close frame to the other side, so the server might decide to just drop the connection at this point without a response (unlikely, but possible) in which case you'd get an error from the tcp layer. This would also apply to your second question.

But ideally, you'd send the close-frame to the server, which then initialized an ordered teardown, then tears down the ssl layer (which google usually skips) and then closes the socket.

@ecorm
Copy link
Author

ecorm commented Aug 29, 2023

I have refactored my server logic to not depend on websocket::stream::is_open.

I now also destroy the websocket::stream upon completion of websocket::stream::async_close (regardless of success/failure), so that should take care of closing the underlying TCP socket.

@ecorm
Copy link
Author

ecorm commented Aug 29, 2023

I now also destroy the websocket::stream upon completion of websocket::stream::async_close (regardless of success/failure), so that should take care of closing the underlying TCP socket.

Oops, I think that would violate this note from the websocket::stream destructor documentation:

A stream object must not be destroyed while there are pending asynchronous operations associated with it.

...as the websocket::async_read operation might still be pending.

The Asio sockets are more robust in this respect, as calling close or the destructor cancels all pending asynchronous operations.

@ecorm ecorm changed the title Postconditions of websocket::stream::async_close are not documented Better document websocket::stream::async_close and teardown relationship Jan 12, 2024
@ecorm
Copy link
Author

ecorm commented Jan 12, 2024

As requested by the author here, I'm bumping this issue so that the relationship between websocket::stream::async_close and teardown is better documented:

  • There's no mention of "teardown" operations in the websocket::stream::close documentation, so one needs to have a least glanced at the entire documentation to be aware that his mechanism exists, and remember that it exists. I have 1485 other things I have to constantly keep in my head, so it's easy for me to forget that websocket::stream::close performs a customizable teardown operation. 😁 Just a note with a link to the Teardown page in the websocket::stream::close documentation should suffice.
  • It's not clear if the teardown operation is completed or is kicked off by time the websocket::stream::async_close handler is invoked.
  • The Teardown documentation page does not describe what the default teardown operation actually does. It sorta implies that the default teardown is per RFC6455 section 7.1.1., but it would be better if this were explicitly stated.
  • It's not clear what state the underlying TCP socket is in if the websocket::stream::close operation fails. With Asio, ip::tcp::socket::close is a no-fail operation and I can count on the socket being actually closed from my perspective.

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