Check wheter atomic ptr is not initialized in gcc 4.4.7 (without nullptr)

518 views Asked by At

During re-factorization of singleton class to be thread safe (fallowing Herb Sutter advice how to write proper double-check locking) I came across problem with my version of compiler (gcc 4.4.7 with --std=c++0x flag) supporting atomics but not supporting nullptr.

Current code

class SingletonClass {
   public:
SingletonClass* getInstance() {
    if (instance == NULL) {
        instance == new SingletonClass();
    }
    return instance;
}

   private:
SingletonClass() = default;
~SingletonClass() = default;

static SingletonClass* instance;};

What I would like to achive

#include <cstdatomic>
#include <mutex>

class SingletonClass {
   public:
SingletonClass* getInstance() {
    if (instance == NULL) {
        std::lock_guard<std::mutex> lock (m);
        if(instance == NULL){
           instance = new SingletonClass();
        }
    }
    return instance;
}

   private:
 SingletonClass() = default;
~SingletonClass() = default;

static std::atomic<SingletonClass*> instance;
static std::mutex m;
};

But this gives me error saying that there is no operator for comparing atomic ptr to NULL

main.cpp: In member function ‘SingletonClass* SingletonClass::getInstance()’:
main.cpp:7: error: ambiguous overload for ‘operator==’ in ‘SingletonClass::instance == 0l’
main.cpp:7: note: candidates are: operator==(void*, void*) <built-in>
main.cpp:7: note:                 operator==(SingletonClass*, SingletonClass*) <built-in>

Since I can't either compare instance ptr to NULL or use nullptr how do I work around it and check whether it is initialized or not ?

2

There are 2 answers

2
Kevin Anderson On

Edit: I think Yksisarvinen's answer above is better. You already have the lock, don't bother anymore. Leaving my old answer below.

I'm not sure about the specific GCC version, but if <atomic> is there theoretically this should work:

SingletonClass* getInstance() {
    if (instance.load() == NULL) {
        std::lock_guard<std::mutex> lock (m);
        if(instance.load() == NULL){
            instance = new SingletonClass();
        }
    }
    return instance;
}

As per the documentation, load() should give you what you want, the pointer itself to compare to, which should work with NULL.

Good luck on old compilers. I've had to do that too on occasion due to customer requirements.

1
Yksisarvinen On

You can use implicit conversion of pointer to bool:

SingletonClass* getInstance() {
    if (instance.load()) {
        std::lock_guard<std::mutex> lock (m);
        if(instance.load()){
           instance.store(new SingletonClass());
        }
    }
    return instance;
}

See it online

Note that implicit conversion from std::atomic<SingletonClass*> to SingletonClass* is possible, but ambiguous in this context. Also, the assignment is ambiguous itself, so store() call was added.


However, perhaps the solution is simpler - why do you need std::atomic at all? You are already locking access to the stored pointer, so you are safe:

#include <mutex>

class SingletonClass {
public:
    SingletonClass* getInstance() {
        std::lock_guard<std::mutex> lock (m);
        if (instance == NULL) {
            instance = new SingletonClass();
        }
        return instance;
    }

private:
    SingletonClass() = default;
    ~SingletonClass() = default;

    static SingletonClass* instance;
    static std::mutex m;
};

std::atomic is used for lockfree access (or at least "hidden lock" access). I can't think of a reason to use both together off the top of my head.

Mutex here is pretty much required - you want to lock whole function as a critical section, or else two threads could create two singleton objects and one would be leaked.