Commits on Jan 31, 2024

1. Fix incorect connection metrics tracking on instances pending shutdown …

   While a spectator server instance was pending shutdown, it could start
   reporting endlessly rising user counts until its actual shutdown. The
   reason for this was questionable logic in `ConcurrentConnectionLimiter`.
   
   When I wrote that thing I used `try...finally` in `OnConnectedAsync()`
   and `OnDisconnectedAsync()` because I wanted to have multiple returns in
   the body of the method and sitll call `await next()` at the end. However
   I appear to have also forgotten that it means that `await next()` would
   be called _even if the preceding code threw an exception_, and then _the
   exception would be thrown anyway_.
   
   This in the case of `StatefulUserHub`s led to the following sequence of
   events:
   
   1. `ConcurrentConnectionLimiter.OnConnectedAsync()` fires
   2. It dies on its innards, but because of the `finally`, calls `await
      next()` anyway
   3. Inside `await next()` is the hub's connection method, in particular
      `LoggingHub.OnConnectedAsync()`, which increments the datadog
      connected user gauge
   4. Then after the finally, because any exception that was thrown was
      not caught, it will get _rethrown_, interrupting the connection flow
   5. After this, `OnDisconnectedAsync()` will not get called for this
      connection, leading the counter to not decrement back down from the
      increment in point (3)
   
   leading to the runaway increase in reported user counts.
   
   To fix, stop using `try...finally` and split the blocks of code that
   want early-returns to separate methods instead.

   

   bdach committed Jan 31, 2024
   Configuration menu

   • View commit details
   • Copy full SHA for fc2a91a
   • Browse repository at this point

   Copy the full SHA

   fc2a91a View commit details

   Browse the repository at this point in the history