2

I'm using Visual Studio 2013, which doesn't have "magic statics" feature implemented yet, so local static variables initialization isn't yet thread-safe. So, instead of

Foo& GetInstance()
{
    static Foo foo;
    return foo;
}

I do something like this:

std::unique_ptr<Foo> gp_foo;
std::once_flag g_flag;

Foo& GetInstance()
{
    std::call_once(g_flag, [](){ gp_foo = std::make_unique<Foo>(); });    
    return *gp_foo;
}

But I don't like the idea of having gp_foo and g_flag global variables (first, problem with the order of initialization of static variables in different translation units; second, I would like to initialize variables only when we need them, i.e. after first call to GetInstance()), so I implemented the following:

Foo& GetInstance()
{
    // I replaced a smart pointer 
    // with a raw one just to be more "safe"
    // not sure such replacing is really needed
    static Foo *p_foo = nullptr;

    static std::once_flag flag;
    std::call_once(flag, [](){ p_foo = new Foo; });    
    return *p_foo;
}

And it seems to work (at least it passes the tests), but I'm not sure it's thread-safe, because here we have the same potential problem with the initialization of static local variables p_foo and flag in multiple threads. Initialization of raw pointer with nullptr and initialization of std::once_flag seems more innocent than calling Foo's constructor, but I would like to know whether it is really safe.

So, are there any problems with the last code snippet?

undermind
  • 1,779
  • 13
  • 33
  • https://codereview.stackexchange.com/ – Blacktempel Sep 13 '17 at 08:17
  • See [call_once on cppreference](http://en.cppreference.com/w/cpp/thread/call_once); specifically bullet point 2. – Simple Sep 13 '17 at 08:29
  • @Simple _No invocation in the group returns before the above-mentioned execution of the selected function is completed successfully, that is, doesn't exit via an exception._, but my concerns are not about std::call_once, but rather about these two lines: `static Foo *p_foo = nullptr; std::once_flag flag;` – undermind Sep 13 '17 at 08:36
  • 1
    Those two variables are zero-initialised as they are `static` variables. They have already been initialised in the data segment of the actual executable. – Simple Sep 13 '17 at 09:32
  • @Simple Oh, indeed. Raw pointer is zero-initialized. But what about `once_flag`? Its default constructor is called. By the standard, this constructor is `constexpr`, but I see that in VS2013 it is not declared as `constexpr`, so do we have a dynamic initialization of a static variable in such case? – undermind Sep 13 '17 at 09:54
  • In 2013 there will probably be dynamic initialisation for the `once_flag`, but note that zero-initialisation still happens even for static objects that will be initialised during startup (i.e. `flag` is zero'd out and then the constructor is called). If you check the source code it sets the member to `_ONCE_FLAG_CPP_INIT` (in 2012, don't have 2013 installed) which is 0, so it's probably safe. – Simple Sep 13 '17 at 10:11
  • @Simple, yes, initially it would be filled with zeroes, but what would happen when 2 threads call `once_flag`'s default constructor at runtime simultaneously? If the initialization of local static variables is not thread-safe and looks like [this](https://stackoverflow.com/a/5567571/3133604) I can imagine the situation when both threads enter the body of _if_ condition, then thread 1 initializes once_flag for the first time, exits the _if_ condition, performs `call_once`, and after then second threads again initializes `once_flag` and again calls `call_once` (since `once_flag` is zero again). – undermind Sep 13 '17 at 10:28

3 Answers3

1

By far the most stable approach to singleton object initialisation is the schwartz_counter. It's how std::cin, cout etc are implemented and how they always work, regardless of initialisation order of global objects.

It works in all versions of c++.

https://en.wikibooks.org/wiki/More_C%2B%2B_Idioms/Nifty_Counter

Richard Hodges
  • 68,278
  • 7
  • 90
  • 142
  • BTW, is it guaranteed that the initialization of all global variables at load time is performed from the same thread? What if we have 2 [Delay-Loaded DLLs](https://msdn.microsoft.com/en-us/library/151kt790.aspx), say, _lib1.dll_ with function _func1_ and _lib2.dll_ with function _func2_, and _func1_ and _func2_ are called simultaneously from different threads? – undermind Sep 13 '17 at 11:04
  • In any case, it doesn't really make this idiom unuseful, since we can safely replace `int` with `std::atomic`, so the first question is just out of curiosity. – undermind Sep 13 '17 at 11:21
  • @undermind schwartz-counted objects are initialised after zeroing of uninitialised globals and before main is run. In a DLL it will happen just before DllMain is run, which I believe happens before all the thread attachment code. But I'd have to check as I haven't used DLLs in a long time. Can't stand them! – Richard Hodges Sep 13 '17 at 11:31
0

If Foo& GetInstance() is only part of the same compilation unit then the initialization order is defined hence it is thread safe.

However, if above is not the case and multiple compilation units are referencing that then the initialization order would depend on the order of the calls to Foo& GetInstance() and if multiple threads are involved then the order is undefined hence not thread safe.

Worth checking:

Griffin
  • 716
  • 7
  • 25
  • But in the last snippet I have only local static variables. – undermind Sep 13 '17 at 08:41
  • Yes, but same can be true/reproducible for two threads calling `Foo& GetInstance()` at the same time. – Griffin Sep 13 '17 at 08:48
  • @undermind having said that it [looks like](https://stackoverflow.com/questions/8102125/is-local-static-variable-initialization-thread-safe-in-c11) in `c++11` this maybe thread-safe. Now I am not very sure, maybe someone else can elaborate. – Griffin Sep 13 '17 at 09:31
0

Your last code snippet is fine from a thread-safe initialization point of view.

However, it is not clear how are you going to use the Foo object in threads calling GetInstance. Since you are returning a reference to a non-const object, I suppose threads may modify the Foo object. Keep in mind that you need additional synchronization for this (eg. a mutex)

If the Foo object is fully initialized by its constructor and threads calling GetInstance will only read from the object, there is no problem but I would suggest to return const Foo &

LWimsey
  • 6,189
  • 2
  • 25
  • 53