Possible `int` overflow in `bmqt::UriParser`

[678098]

Is there an existing issue for this?

• I have searched the existing issues

Current Behavior

No response

Expected Behavior

No response

Steps To Reproduce

However, I can see one very unprobable scenario with a possible bug here.

1. Within UriParse::initialize we increment int s_initialized counter in a thread-safely manner (within the locked mutex).
   

   blazingmq/src/groups/bmq/bmqt/bmqt_uri.cpp

   Line 195 in 342b608

   ++s_initialized;

2. We early return if this counter is greater than 1, meaning, that some other code has already incremented it and initialized a shared state which is const-read-thread-safe.

3. However, we do bmqt::UriParse::initialize every time we call a constructor for bmqt::Uri, and do not deinitialize bmqt::UriParser until we call a destructor bmqt::Uri::~Uri.

4. This means, that if we are able to keep approx 2^31 bmqt::Uris in memory, we'll get int overflow.

5a. Because of this possible overflow, the condition for early exit will fail on a new bmqt::Uri construction, and we might re-initialize some const-read-thread-safe RegEx, when some other thread might use it

5b. It is also possible to just launch bmqt::UriParser::initialize 2.2kkk times to get overflow. Even if we do call bmqt::UriParser::shutdown the same number of times afterwards (according to contract), there will be a problem.

BlazingMQ Version

0.90.21

Anything else?

There are 2 possible solutions.

1. The minimal change (and safe) solution is to change the counter type int -> bsls::Types::Int64, so it will be impossible to reach overflow.

2. The good solution is to give up on a custom reference counting of bmqt::UriParser::initialize and initialize it once and for all as a singleton, using the appropriate tool to initialize: BSLMT_ONCE_DO https://bloomberg.github.io/bde-resources/doxygen/bde_api_prod/group__bslmt__once.html

The greatest benefit of this approach is that we don't lock the mutex every time we need to build a bmqt::Uri. As you can see here (https://github.com/bloomberg/bde/blob/88f20d8057d462e2bdaf27b757ff3bf335555dcd/groups/bsl/bslmt/bslmt_once.cpp#L25), bslmt::Once can check the need to do something without locking the internal mutex, when the state of once was already decided. As a result, we'll be able to work with bmqt::Uris much, much faster from different threads.

The difference: we'll not free these resources gracefully to make the allocator happy in the end of the program. I don't think this difference is bad, because the needed allocated memory is small, we already have some singletons allocated like this, and the runtime performance gain is huge.

We can also deprecate bmqt::UriParser::initialize, bmqt::UriParser::shutdown or make them no-op.