2

I have a WebSocket server done with Boost.ASIO and Boost.Beast. It follows the idiomatic ASIO design: session (connection) objects own the communication socket and derive from std::enable_shared_from_this. Async completion handlers capture a std::shared_ptr to self keeping the object alive while there're pending operations, and the objects get destructed automatically when the chain of async ops end. The io_context runs on a single thread, so everything is in an implicit strand.

All this is fairly simple when there's only one chain of async handlers. The session objects I have contain an additional TCP socket and a timer. Read operations are concurrently pending on 2 sockets forwarding messages back and forth, while the timer runs periodically to clean up things. To kill such an object I created a destroySession method that calls cancel an all resources, and eventually completion handlers get called with operation_cancelled. When these all return without scheduling any new async op, then the object gets destructed. destroySession calls are carefully placed at every location where a critical error happens that should result in session termination.

Question1: Is there better way to destruct such an object? With the above solution I feel like I'm back 90's where I forget a delete somewhere and I got a leak...

Given that all destroySession calls are there, is it still possible to leak objects? In some test envs I see 1 session object in 1000 that fails to destruct. I'm thinking of a similar scenario:

  • websocket closure and timer expiry happens at the same time
  • websocket completion handler gets invoked, timer handler enqueued
  • websocket completion handler cancels everything
  • timer expiry handler gets called (not knowing the error) reschedules the timeout
  • timer cancel handler gets invoked and simply returns, object remains alive (by the timer)

Is this scenario plausible?

Question2: After calling cancel on a timer/socket can ASIO invoke an already enqueued completion handler with other status than operation_cancelled?

Gyorgy Szekely
  • 2,654
  • 3
  • 23
  • 32
  • ASIO is just helping to make asynchronous communication channels between two active parties. It is like mouth and ears. Whatever issues with it are in parts that do the actual communicating written by you. It is like brain. Mouth and ears can not fix issues in mad brain. – Öö Tiib Dec 05 '22 at 12:18
  • @ÖöTiib yeah but isn't that just the question? How to design a good brain.. – Jakob Stark Dec 05 '22 at 12:20
  • To me the scenario with two simultaneous handlers seems plausible. You could have a per-session atomic boolean variable, that is set in `destroySession` before calling the cancels. Then make sure to never reschedule anything if that variable is set in your handlers. – Jakob Stark Dec 05 '22 at 12:25
  • @JakobStark Good brain emerges slowly over time by calling abort() each time when it realizes its own madness. The destroySession on lot of things is itself kind of bad case of autism. – Öö Tiib Dec 05 '22 at 12:31

1 Answers1

0

Nice description. Even though code is missing, I have a very good sense of both your design and your understanding of Asio. Both of which seem fine :)

First thoughts:

  1. I kind of agree with the sentiment that destroySession might be a code smell of itself. I can't really state it for lack of details. In my code, I make sure to cancel the "complementary async chain", not just a broad cancel of everything. And the need rarely arises outside the common case of a async timer.
  2. Also, I'm a little worried about the vague "timer runs periodically to clean up things" - in the sketched design there is nothing to clean up, so I worry whether the things you're not showing (leaving out of the description) might cause the symptoms you're trying to explain.

The Timer Scenario

Yes, this is a plausible scenario. In fact it's a bit of a common pitfall problem with Asio timers:

Cancelling boost asio deadline timer safely

SUMMARY TL;DR

Cancelling a time only cancels asynchronous operations in flight.

If you want to shutdown an asynchronous call chain, you'll have to use additional logic for that. An example is given below.

The answer goes into detail how to trace cases like this, and also a approach to fix it.

sehe
  • 374,641
  • 47
  • 450
  • 633