Double-checked locking in C++11?

Question

Here is a Java example problem from http://www.ibm.com/developerworks/java/library/j-dcl/index.html

public static Singleton getInstance()
{
  if (instance == null) //#4
  {
    synchronized(Singleton.class) {  //#1
      if (instance == null)          //#2
        instance = new Singleton();  //#3
    }
  }
  return instance;
}

It seems this isn't safe because #3 can set instance to not null before the constructor is executed thus when another thread checks instance on #4 it will not be null and return a instance that hasn't been constructed properly.

Apparently using a function variable wont help because it may be optimized out or just be executed in a way that also sets the value to instance when we don't want it to.

I was thinking wouldn't the easiest way is to have a function new Singleton(); so it is completed before assigning it to instance. Now the issue is how do I tell C++ a function should NOT be inline? I think Singleton* make_singleton() volatile should do that but i'm positive I am wrong.

I'm a little confused. The article and code in your question are about Java, and then you're talking about C++11. Are you trying to figure out how to implement this idiom in C++? If so, the article you linked already talks about why the Java version won't work. What does your C++ function look like? — Gian, May 07 '13 at 06:27
I'm afraid you'll just keep attracting downvotes from people who lack reading comprehension and think that you're talking about Java. — congusbongus, May 07 '13 at 06:32
@GManNickG: :D I NEVER do. It just bugged me that `new` might have a problem when working with threads — , May 07 '13 at 06:39

R. Martinho Fernandes · Accepted Answer · 2013-05-10T09:15:55.060

27

I'll ignore the singleton bits for a while and assume you need this for lazy initialization and not for silly things like singletons.

I suggest forgetting about double-checked locking. C++ provides a very useful tool for this kind of situation in the form of std::call_once, so use that.

template <typename T>
struct lazy {
public:
    // needs constraining to prevent from doing copies
    // see: http://flamingdangerzone.com/cxx11/2012/06/05/is_related.html
    template <typename Fun>
    explicit lazy(Fun&& fun) : fun(std::forward<Fun>(fun)) {}

    T& get() const {
         std::call_once(flag, [this] { ptr.reset(fun()); });
         return *ptr;
    }
    // more stuff like op* and op->, implemented in terms of get()

private:
    std::once_flag flag;
    std::unique_ptr<T> ptr;
    std::function<T*()> fun;
};

// --- usage ---

lazy<foo> x([] { return new foo; });

edited May 10 '13 at 09:15

answered May 07 '13 at 14:18

R. Martinho Fernandes

228,013
71
433
510

I would suggest using `call_once` rather than `std::call_once`. It doesn't make a difference here, but one should generally prefer to call functions unqualified to allow ADL to take effect. Since `flag` is of type `std::once_flag`, namespace std will be an associated namespace, making the `std::` unnecessary. – Adam H. Peterson May 07 '13 at 15:25
23

Erm. No. One should generally prefer whatever achieves one's goal. I want to call `std::call_once` and only that, *not anything else*, making `std::` absolutely *crucial* to ensure that. – R. Martinho Fernandes May 07 '13 at 15:29
1

I agree that works in this case, and it's mostly a style thing. However, I see a lot of people call `std::swap`, which is usually wrong (especially in template code) because it suppresses ADL. That's why I _suggest_ always calling functions unqualified, unless there's a specific need to control exactly which function is called and ADL would do the wrong thing. Since here ADL will pick `std::call_once`, my recommendation is to not suppress ADL. – Adam H. Peterson May 07 '13 at 16:15
GREAT answer! I never use singletons but knowing this is available I can kick the ass out of anything that should have lazy evaluating. Between this and the other answer (atomics wont be reordered) I have not much worries about multithreading – May 07 '13 at 20:36
@AdamH.Peterson when you want ADL with an `std` function as a backup you will also include a using statement, such as `using std::swap; swap(a, b);`. Unless you want to additionally recommend `using std::call_once; call_once(flag);` then I wouldn't think the "it's the proper way to write generic code" is a good reason to prefer unqualified names. I sometimes prefer unqualified names even when I'm writing non-generic code simply because I find it shorter and more readable. – bames53 May 10 '13 at 03:26
shouldn't `call_once` lambda capture `this` instead of `&ptr`? – boro May 10 '13 at 07:29
8

@AdamH.Peterson the case for unqualified calling `swap` is quite different from that of other functions from the Standard Library. `swap` could be part of a user-defined class interface and generic code should find it, and fall back to `std::swap` otherwise. But this almost never applies to any other algorithm from the Standard Library, and they should always be called with `std::`. Especially if you are a library author, not using `std::` introduces unintended points of customization (your code will subtly break if users define functions with the same name). – TemplateRex May 10 '13 at 12:32
Is this safe if called from multiple threads? Couldn't `ptr.reset` be executing at the same time as `*ptr`? – David May 10 '13 at 15:17
1

@Dave no, that cannot happen. See the docs for `call_once`: "No invocation in the group returns before the abovementioned execution of the selected function is completed successfully, that is, doesn't exit via an exception." – R. Martinho Fernandes May 10 '13 at 15:21

score 2 · Answer 2 · answered May 07 '13 at 14:11

This is exactly the type of situation atomics are designed for. By storing the result into an atomic, you know that the compiler can't sequence any critical stores or operations after the atomic is set. Atomics are both for emitting processor instruction primitives to ensure the necessary sequential consistency (e.g., for cache coherency across cores) and for telling the compiler which semantics must be preserved (and therefore to limit the types of reorderings it can do). If you use an atomic here, it won't matter if the function is inlined because whatever inlining the compiler does will have to preserve the semantics of the atomic itself.

You may also be interested in looking into std::call_once which is also designed for this situation, and more specifically for the situation where multiple threads may need something done, but exactly one of them should do it.

Double-checked locking in C++11?

2 Answers2