2

I implemented a Meyers Singleton, then realized it could be vulnerable to the destructor fiasco problem.

As a result, I changed the code to be:

Instance *getInstance()
{
    static Instance* singleton = new Instance();
    return singleton;
}

After implementing this, and no apparent bugs occuring, a coworker was implementing a different singleton and used std::call_once instead.

I've now realized that after much searching, I couldn't find if the "Leaky Meyers Singleton" is a threadsafe pattern. Should the leaky singleton be changed to std::call_once? Or is it threadsafe as-is?

Is the pointer considered a "block-scope" variable? If so I think it would be thread-safe, but if not there's significant bugs introduced with the current leaky singleton approach.

ListsOfArrays
  • 442
  • 6
  • 14
  • Thread-safety doesn't depend on whether it's a pointer or not, so you should be safe. – HolyBlackCat Jan 09 '22 at 17:38
  • 1
    Why not ` static Instance singleton;` – JVApen Jan 09 '22 at 17:41
  • 4
    [Meyer's Singleton](https://stackoverflow.com/a/1008289/3807729) is indeed thread safe (after `C++11`). I think there are only some obscure circumstances when destructor fiasco could be an issue. For that you could use `std::shared_ptr` https://stackoverflow.com/a/40337728/3807729 – Galik Jan 09 '22 at 17:41
  • 2
    If you follow @JVApen 's suggestion I don't think you will suffer from a destructor fiasco unless you have some other instance of something alive that depends on the above `Instance` to be alive after it's been destroyed ... – Ted Lyngmo Jan 09 '22 at 17:49
  • 1
    Your code has an unneeded new in it meyer's singleton returns a reference. *Instance& getInstance() { static Instance instance; return instance; }* Your code will be detected by many tools as a memory leak since the singleton instance will never be deleted (there is no matching delete call). The version returning the reference WILL call the destructor of Instance at shutdown of your program – Pepijn Kramer Jan 09 '22 at 18:03
  • The best suggestion is to stop using singletons – Taekahn Jan 09 '22 at 18:20
  • @Taekahn Well I would say use with extreme care. But indeed I would recommend designs using dependency injection (at least if you want to be able to write unit tests) – Pepijn Kramer Jan 09 '22 at 19:25
  • @pepijn Valgrind and Address Sanitizer both don't report it as an error. It seems probable they support the leaky singleton problem. – ListsOfArrays Jan 10 '22 at 15:38
  • There were reasons singletons were chosen. There is a color wrapper for standard out where DI doesn't help the nature of the test and the added parameters polluted the API everywhere... The other singleton was to centralize error handling and safe application shutdown across multiple threads. DI started being a nightmare to maintain for that, so a singleton it became. – ListsOfArrays Jan 10 '22 at 15:43
  • @Ted as it's a global error handling singleton (simplified for post ofc), there's a large possibility that another static object of indeterminate initialization will in the future depend on it when the static destructors fire off. I have dealt with static destructor issues in the past (double symbol include from SO and main binary) and never want them in the future lol. All you get is a symbol error with no stack trace, it's horrific. – ListsOfArrays Jan 10 '22 at 15:48
  • 1
    @ListsOfArrays It's so long ago I used singletons with a pointer but I remember MSVC debug builds reporting it as undeleted memory thats why :) And I can relate to DI polluting an constructors too (usually I create some kind of DI context structure for and put interfaces in that and pass those around) In the end its all engineering and choosing appropiate tradeoffs. Anyway seems you've giving it more then enough thought! – Pepijn Kramer Jan 10 '22 at 16:18

1 Answers1

3

Yes, this is thread-safe: no two threads will ever try to initialize the same variable with static storage duration at the same time. That includes the entirety of evaluating the initializer.

Davis Herring
  • 36,443
  • 4
  • 48
  • 76