c++: is this getInstance() method of my singleton threadsafe?

Question

I am currently trying to write a threadsafe singleton (at least in terms of construction and destruction) using boost::mutex. I read that boost mutex can't be initialized statically(I lost the link where I read it, sorry) so to work around that I tried this, if threadsafety for construction and destruction is:

                static T& getInstance()
            {
#ifndef STATIC_VARIABLES_ARE_THREADSAFE
                boost::mutex mutex; 
                boost::lock_guard lock(mutex);
#endif
                static T instance;
                return instance;
            }

is that threadsafe, or should I use boost::call_once? Will boost once give me any performance benefit over this approach?

EDIT: Okay, my first idea was obviously not correct. To clearify the question. Can a boost::mutex safely be initialized statically? Like this:

class Singleton
{
private:
static boost::mutex m_mutex;

public:
static Singleton & getInstance()
{
    boost::lock_guard lock(m_mutex);
    static T instance;
    return instance;
}

};

Is that a working approach or is it in fact not safe to init boost::mutex statically (which is what I read)?

EDIT2: Ah, that was the link by the way http://uint32t.blogspot.com/2007/12/you-lazy-bastard-part-1.html

Did you check this question: efficient thread-safe singleton in C++ (http://stackoverflow.com/questions/2576022/efficient-thread-safe-singleton-in-c) — yasouser, May 24 '11 at 20:39
yes, but my question is rather if boost mutex can be statically initialized, because if thats the case, the problem could be solved pretty easily. — moka, May 24 '11 at 20:41
Singletons are evil, do not use them unless you really... REALLY have to :) — , May 24 '11 at 21:00

score 3 · Accepted Answer · answered May 24 '11 at 20:36

3

Since each entry to the function will create its own lock, the lock will be completely useless; any number of threads can enter the function, lock different locks, and start messing with the static data at the same time.

You could create the lock at file (or class static) scope; this would ensure it's created in time, provided that you don't start threads before main(). However, this would also serialize entry to the function even after the static data is initialized.

Why not just define your static data at file (or class static) scope in the first place?

answered May 24 '11 at 20:36

bdonlan

224,562
31
268
324

1

now that you say it, it's pretty obvious, thanks! I guess I have to go down the boost/std::call_once route then. Is it true that boost mutex can't be initialized statically? – moka May 24 '11 at 20:38
Well, can I safely initialize a boost mutex statically? As stated I read somewhere that this is not the case, so I am unsure if that is a valid approach. see updated post! – moka May 24 '11 at 20:56
2

It's not safe to initialize it as a function-scope static variable; this is because two threads entering the function may try to initialize the mutex itself at the same time. As a class-level static, it's safe as long as you don't access it before static initialization finishes (this is a problem with any static non-POD type - see http://www.parashift.com/c++-faq-lite/ctors.html#faq-10.14 ) – bdonlan May 24 '11 at 21:00
2

@bdonlan : Note that while that's correct for C++03, in C++11 static locals are guaranteed to be initialized by the runtime in a thread-safe manner. – ildjarn May 24 '11 at 21:07
1

That's a certainly an option. – bdonlan May 24 '11 at 21:08
thats why I am using a #ifndef STATIC_VARIABLES_ARE_THREADSAFE check. I will simply use a class-level static mutex for now if threadsafe initialization of static variables is not guaranteed since that seems to be safe enough. – moka May 24 '11 at 21:11

score 1 · Answer 2 · answered May 25 '11 at 18:26

1

GCC provides thread safe initialization of static variables. So, if you use GCC you do not need to care about proper initialization of static variables

answered May 25 '11 at 18:26

DimanNe

1,791
3
12
19

score 0 · Answer 3 · answered Jan 21 '20 at 20:46

Try to use the Meyers Singleton implementation.

class MySingleton {
public:
  static MySingleton& getInstance() {
    static MySingleton instance;
    return instance;
  }
private:
  MySingleton();
  ~MySingleton();
  MySingleton(const MySingleton&)= delete;
  MySingleton& operator=(const MySingleton&)= delete;

};

MySingleton::MySingleton()= default;
MySingleton::~MySingleton()= default;


// Usage
int main() {
  MySingleton::getInstance();
}

More info...

score 0 · Answer 4 · answered May 24 '11 at 20:38

0

You do need to use locking. However, the boost::mutex scheme you propose is broken since you allocate the mutex on the stack and multiple concurrent callers would each get their own mutex.

answered May 24 '11 at 20:38

NPE

486,780
108
951
1,012

c++: is this getInstance() method of my singleton threadsafe?

4 Answers4