c++websocketboostboost-asioboost-beast

Call async_ping and/or async_close when there is an incompleted async_write operation


The boost documentation has me confused.

The boost 1.68.0 documentation for websocket::stream::async_ping says:

This operation is implemented in terms of one or more calls to the next layer's async_write_some functions, and is known as a composed operation. The program must ensure that the stream performs no other writes until this operation completes.

https://live.boost.org/doc/libs/1_68_0/libs/beast/doc/html/beast/ref/boost__beast__websocket__stream/async_ping.html

The boost 1.68.0 documentation for websocket::stream::async_close says:

This operation is implemented in terms of one or more calls to the next layer's async_write_some functions, and is known as a composed operation. The program must ensure that the stream performs no other write operations (such as websocket::stream::async_ping, websocket::stream::async_write, websocket::stream::async_write_some, or websocket::stream::async_close) until this operation completes.

https://live.boost.org/doc/libs/1_68_0/libs/beast/doc/html/beast/ref/boost__beast__websocket__stream/async_close.html

In my other question I got an answer that also says that async_ping, async_close and async_write operations should not compete with each other, but should be performed in turn, see Correct use of ping/async_ping with websockets

It would seem that everything is clear, but the more recent boost documentation confused me. The thing is that the newer boost documentation (1.81.0) doesn't mention that async_ping, async_close and async_write calls shouldn't compete. It's just not in the documentation, see:

  1. async_close: https://live.boost.org/doc/libs/1_81_0/libs/beast/doc/html/beast/ref/boost__beast__websocket__stream/async_close.html
  2. async_ping: https://live.boost.org/doc/libs/1_81_0/libs/beast/doc/html/beast/ref/boost__beast__websocket__stream/async_ping.html

So who is right? Old boost documentation and this answer https://stackoverflow.com/a/79667731/25493371 or new boost documentation?


Solution

  • First off, the wording "should not compete" is unnecessarily confusing. Whether operations compete is not normative, but factual: either operations compete, or they don't. Competing operations MUST NOT overlap.

    With that out of the way,


    Note that the following verbiage is still there:

    The algorithm, known as a composed asynchronous operation, is implemented in terms of calls to the next layer's async_write_some function

    This in and of itself could be enough to outlaw triggering other async_write_some operations.

    The only exception I was previously aware of is for writes that may originate from receiving control frames while reading a message: Control Frames

    During read operations, Beast automatically reads and processes control frames. If a control callback is registered, the callback is notified of the incoming control frame. The implementation will respond to pings automatically. The receipt of a close frame initiates the WebSocket close procedure, eventually resulting in the error code error::closed being delivered to the caller in a subsequent read operation, assuming no other error takes place.

    A consequence of this automatic behavior is that caller-initiated read operations can cause socket writes. However, these writes will not compete with caller-initiated write operations. For the purposes of correctness with respect to the stream invariants, caller-initiated read operations still only count as a read. This means that callers can have a simultaneously active read, write, and ping/pong operation in progress, while the implementation also automatically handles control frames.

    Note that it merely extends the "duplex" norm (0 or 1 active write and 0 1 active read) to pings: (0/1 active operation of each of the three kinds at a time).

    Did Anything Change?

    I consulted the Release Notes, which doesn't explicitly state a behaviour change.

    1.70

    • websocket::stream supports

      • Configurable handshake timeout
      • Configurable idle timeout
      • Automatic idle pings

    1.67

    At that stage we're already past the 1.68 release you took as reference. Nothing here explicitly claims a change in operation restrictions.

    When Did The Wording Change?

    The wording change is for issue #1213, which spotted the same contradiction in the Notes/Thread Safety documentation.

    Consequently, the docs have been clarified.

    Where it used to say:

    The asynchronous interface supports one active read and one active writeAdd commentMore actions simultaneously. Undefined behavior results if two or more reads or two or more writes are attempted concurrently. Caller initiated WebSocket ping, pong, and close operations each count as an active write.

    It now says:

    Like a regular Boost.Asio socket, a stream is not thread safe. Callers are responsible for synchronizing operations on the socket using an implicit or explicit strand, as per the Asio documentation. The websocket stream asynchronous interface supports one of each of the following operations to be active at the same time:

    For example, the following code is malformed, because the program is attempting to perform two simultaneous reads:

       ws.async_read(b, [](error_code, std::size_t){});
       ws.async_read(b, [](error_code, std::size_t){});
    

    However, this code is well-formed:

       ws.async_read(b, [](error_code, std::size_t){});
       ws.async_write(b.data(), [](error_code, std::size_t){});
       ws.async_ping({}, [](error_code){});
       ws.async_close({}, [](error_code){});
    

    The implementation uses composed asynchronous operations; although some individiual operations can perform both reads and writes, this behavior is coordinated internally to make sure the underlying stream is operated in a safe fashion. This allows an asynchronous read operation to respond to a received ping frame even while a user-initiated call to asynchronous write is active.

    Since then, the wording has been further simplified, replacing malformed with produces undefined behavior and wellformed with correct.

    Conclusion

    Yes, that restriction was loosened. I wasn't aware of this documentation change when it happened, so thanks for bringing it to my attention.