0

I came over this question recently, and got doubts about the Instance() function implementation:

class Configuration
{
public:
    static Configuration* Instance() {
        static Configuration * myInstance = new Configuration();
        return myInstance;
    }

    int i;
    // delete copy and move constructors and assign operators
    Configuration(Configuration const&) = delete;             // Copy  construct
    Configuration(Configuration&&) = delete;                  // Move construct
    Configuration& operator=(Configuration const&) = delete;  // Copy assign
    Configuration& operator=(Configuration &&) = delete;      // Move assign

protected:
    Configuration() {

    }
    ~Configuration() {}

    // ...
}

Unfortunately the OP doesn't seem to be able to provide a MCVE that reproduces that read access violation they claim.

  • Is using an instance pointer and new in that implementation still guaranteed to be thread safe (a race condition could be a potential reason for that error)?

Here's an example of the working code, there's only a single thread involved though.

  • There's nothing special about `new` here. The same answer that applies to examples that don't use `new` also apply here. The error seen in the linked question is likely caused by unrelated undefined behavior. – François Andrieux Jan 22 '18 at 20:34
  • @François Can you provide a reference for that? I wasn't sure about the atomicity of the constructor execution using _new_. –  Jan 22 '18 at 20:38
  • While `new` itself is thread-safe, (https://stackoverflow.com/questions/796099/c-new-operator-thread-safety-in-linux-and-gcc-4) it's the assignment that matters. – MFisherKDX Jan 22 '18 at 20:39
  • 1) threads; thread carefully. Very, very carefully. 2) singletons; pain and madness lies there - just say no. 3) combine threads and singletons? Are you nuts? – Jesper Juhl Jan 22 '18 at 20:40
  • 3
    There isn't much to add beyond [this question](https://stackoverflow.com/questions/1661529/is-meyers-implementation-of-the-singleton-pattern-thread-safe). Rather you initialize with `new` or from some other source, the initialization will happen once in a thread-safe manner. – François Andrieux Jan 22 '18 at 20:40
  • @Jesper Sure, I well know and would never use that like this in my own code ;-). –  Jan 22 '18 at 20:41
  • 1
    `myInstance` is an object with static storage duration. Its initialization will be done in a thread safe manner. Doesn't matter that the initializer is a new expression. Pointers have rights too :) – StoryTeller - Unslander Monica Jan 22 '18 at 20:42
  • 2
    One of the benefits of using a `static` local as a singleton is that it will be destroyed automatically. By using a raw owning pointer and dynamic allocation, you throw that away. Even if you need dynamic allocation, `static auto myInstance = std::make_unique();` would be preferable. – François Andrieux Jan 22 '18 at 20:43
  • 1
    @FrançoisAndrieux You might as well just use `static Configuration myInstance; return &mInstance;` at that point. –  Jan 22 '18 at 20:45
  • @Jesper _"Are you nuts?"_ Surely not. One of the guarantees given with Scott's idiom is that the initialization is thread safe since c++11, vs the lock and double check technique used in older _"idioms"_ which use a `static` class member variable for the instance. I'm also not bothering about the general usefulness of the _Singleton Pattern_ here. Just referring to the particular case. –  Jan 22 '18 at 20:46
  • @user9212993 It is still nuts, because a) it is a singleton an b) you will have to synchronise every non-const access. Also you have a memory leak. – juanchopanza Jan 22 '18 at 20:55
  • @juanchopanza I don't say this is good code, and it's not mine. So why are you calling ***me*** _"nuts"_ actually? This is a decent question. –  Jan 22 '18 at 20:57
  • @user9212993 I wouldn't be so sure. If you want to know if initializing a local static pointer from a `new` expression is thread safe, then you don't need a singleton at all. Just a plain function with a local static will do. Your question is mainly noise. – juanchopanza Jan 22 '18 at 21:04
  • @juanchopanza I'm well aware that using a _Singleton_ isn't necessary, and surely would avoid that for any purpose. My question asks just what it asks about. Feel free to hammer that as a dupe of https://stackoverflow.com/questions/1661529/is-meyers-implementation-of-the-singleton-pattern-thread-safe –  Jan 22 '18 at 21:08
  • @jesper BTW: I managed myself to do _thread safe_ and properly working implementations of _Singletons_. It probably could be that your TV broadcast provider runs that software 24/7. So what? And yes, for these cases there were good reasons to use a singleton application instance. Don't try to diminish me, just because I have low reputation here! –  Jan 22 '18 at 21:17

2 Answers2

1

Is using an instance pointer and new in that implementation still guaranteed to be thread safe (a race condition could be a potential reason for that error)?

Yes, it is thread safe.

From N4659:

9.7 Declaration statement [stmt.dcl]

Dynamic initialization of a block-scope variable with static storage duration (6.7.1) or thread storage duration (6.7.2) is performed the first time control passes through its declaration; such a variable is considered initialized upon the completion of its initialization. If the initialization exits by throwing an exception, the initialization is not complete, so it will be tried again the next time control enters the declaration. If control enters the declaration concurrently while the variable is being initialized, the concurrent execution shall wait for completion of the initialization. If control re-enters the declaration recursively while the variable is being initialized, the behavior is undefined.

As myInstance is a block-scope variable with a static storage duration which is dynamically initialized, the code is thread-safe even if multiple threads are involved.

Edgar Rokjān
  • 17,245
  • 4
  • 40
  • 67
0

Just as FYI, wanted to add the below reference which explains when to use a pointer and when not: https://isocpp.org/wiki/faq/ctors#construct-on-first-use-v2

Basically using a pointer will not leak since the OS will reclaim memory once the process exists, this is likely the best approach in most of the cases. However if the singleton needs to close another resource, i.e. a file, in destructor, then that's a problem. In that case don't use the static local variable as pointer but just a static local object. Make sure that any other static objects that use this object in destructors are used in their constructors as well, to make sure when the program exists, calls their destructors in correct order.

AdvSphere
  • 986
  • 7
  • 15
  • 1
    I didn't ask about _leaking_ but _thread safety_ on 1st access. I don't get well what your answer adds about my question? –  Jan 22 '18 at 21:57
  • Not trying to reply to your question, but adding a reference to François Andrieux comment above. – AdvSphere Jan 22 '18 at 22:03
  • 1
    If you're not _"trying to reply to my question"_, that can't be considered a valid answer, no? A comment maybe, but that was already made, and all that information is already available. –  Jan 22 '18 at 22:05