5

I have this:

Singleton.h

#ifndef SINGLETON_H
#define SINGLETON_H

#include <atomic>
#include <mutex>

class Singleton
{
public:
    static std::atomic<Singleton*> Singleton::m_instance;
    static std::mutex Singleton::m_mutex;
    static Singleton* getInstance();

    Singleton();
    ~Singleton();
};

#endif

Singleton.cpp

#include "Singleton.h"

Singleton::Singleton()
{
}

Singleton* Singleton::getInstance() 
{
    Singleton* tmp = m_instance.load(std::memory_order_relaxed);
    std::atomic_thread_fence(std::memory_order_acquire);
    if (tmp == nullptr) 
    {
        std::lock_guard<std::mutex> lock(m_mutex);
        tmp = m_instance.load(std::memory_order_relaxed);
        if (tmp == nullptr) 
        {
            tmp = new Singleton;
            std::atomic_thread_fence(std::memory_order_release);
            m_instance.store(tmp, std::memory_order_relaxed);
        }
    }
    return tmp;
}

Singleton::~Singleton() {}

main.cpp

#include "Singleton.h"
#include <iostream>
int main()
{
    Singleton* singleton = Singleton::getInstance();
    std::cout << "Hello World!" << std::endl;
    return 0;
}

When i try to build that i get this errors(Visual studios):

Error 1 error LNK2001: unresolved external symbol "public: static struct std::atomic Singleton::m_instance" (?m_instance@Singleton@@2U?$atomic@PAVSingleton@@@std@@A) c:...Singleton.obj Singleton

AND:

Error 2 error LNK2001: unresolved external symbol "public: static class std::mutex Singleton::m_mutex" (?m_mutex@Singleton@@2Vmutex@std@@A) c:\Users\InusualZ\documents\visual ...Singleton.obj Singleton

InusualZ
  • 49
  • 1
  • 5

1 Answers1

7

You need to define the static member variables in the source file, not just declare them in the class definition:

std::atomic<Singleton*> Singleton::m_instance;
std::mutex Singleton::m_mutex;

You may be interested to know that you can achieve almost exactly the same lazy thread-safe initialisation using a simple local static variable:

Singleton* Singleton::getInstance() {
    static Singleton instance;
    return &instance;
}

This fixes the memory leak, but does introduce a potential deathtrap if you try to access it from the destructor of another static variable. There is no way to implement the Singleton antipattern in C++ without some kind of problem. You should think again about whether a singleton is suitable for your design. In my experience, it never is.

Community
  • 1
  • 1
Mike Seymour
  • 249,747
  • 28
  • 448
  • 644
  • i have tried that methods, but i read that in VS don't work corretly – InusualZ Nov 28 '14 at 16:31
  • BTW, why i need to declare does variable twice? – InusualZ Nov 28 '14 at 16:32
  • @InusualZ: You don't need to declare them twice. You need to define them, as well as declaring them as class members. – Mike Seymour Nov 28 '14 at 16:35
  • I really don't get what you mean – InusualZ Nov 28 '14 at 16:37
  • 1
    @InusualZ: You've declared the static members in the class. That tells the compiler that they exist, and are defined somewhere in the program. You must also define them somewhere in the program (in one source file), so that storage can be allocated for them. So put the two lines of code from my first snippet in the `Singleton.cpp` source file, in order to define the variables there. That will fix the link error, which is caused by the missing definition. – Mike Seymour Nov 28 '14 at 16:45