Issue with initialization order for ManagedStaticMutex

We’re seeing an issue with ManagedStaticMutex. There is a race condition between mutex_init_flag and the call_once usage in getManagedStaticMutex, at least on MSVC 2017.

The sequence of operations we see during startup are as follows:

  1. The first global ManagedStatic initializes, causing getManagedStaticMutex to call initializeMutex through std::call_once
  2. Some N number of additional global ManagedStatic initialize, where getManagedStaticMutex skips initializeMutex since it has already been called
  3. The global constructor for “mutex_init_flag” runs, resetting the flag indicating the call has already been made
  4. Another global ManagedStatic initializes, causing getManagedStaticMutex to be called, but this time initializeMutex is run since the mutex_init_flag flag was reset

After (4), the original ManagedStaticMutex is leaked. Additionally, (1) and (2) are operating on an uninitialized global variable.

As a fix, is there anything wrong with making mutex_init_flag function-local in getManagedStaticMutex? Its only use is in that function, and this would guarantee mutex_init_flag is initialized before use.

How does that happen? In my version of MSVC, once_flag has a constexpr constructor, so it shouldn’t use dynamic initialization.

It looks like STL wrote a post about this a while ago:
https://blogs.msdn.microsoft.com/vcblog/2015/07/14/stl-fixes-in-vs-2015-part-2/

He said:
“* once_flag’s constructor wasn’t marked as constexpr as required by the Standard (DevDiv#497946). (Note that while it’s marked as constexpr in VS 2015 RTM, it’s affected by the compiler bug DevDiv#1134662 “constexpr constructors are emitting dynamic initializers”, which we’re planning to fix in 2015 Update 1.)”

I checked, and it looks like MSVC emits a dynamic initializer for std::once_flag globals even though they are marked constexpr. So, LLVM’s code is correct, but there is an MSVC compiler bug. We should probably work around it, though.

I think these days we have thread-safe static locals (check that we don’t disable them with the compiler flag, though, please), so your suggested fix sounds good to me.