beast icon indicating copy to clipboard operation
beast copied to clipboard

Better document `websocket::stream::async_close` and teardown relationship

Open ecorm opened this issue 2 years ago • 5 comments

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 avatar Aug 28 '23 23:08 ecorm

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)?

ecorm avatar Aug 29 '23 00:08 ecorm

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.

klemens-morgenstern avatar Aug 29 '23 05:08 klemens-morgenstern

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 avatar Aug 29 '23 05:08 ecorm

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 avatar Aug 29 '23 06:08 ecorm

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. :grin: 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.

ecorm avatar Jan 12 '24 21:01 ecorm