why my implementation of singleton instatiated twice? (one process, several threads)

Question

I have static library which contains such singleton definition:

    class InstrumentsStorage
    {
    public:
        static InstrumentsStorage& getInstance() {
            static InstrumentsStorage instance;
            return instance;
        }
        // methods
    private:
        InstrumentsStorage();
        InstrumentsStorage(InstrumentsStorage const&);
        void operator=(InstrumentsStorage const&);

        // fields
    };

I've added such tracing:

InstrumentsStorage::InstrumentsStorage() {
    std::cout << "InstrumentsStorage constructor called!" << std::endl;
            ...

And in my logs I find this string twice. Why? How to fix my singleton so only one instance is created. I can use C++11.

I'm using singleton from different projects from different threads but I have only one process.

upd adding full listing:

#pragma once

#include <string>
#include <iostream>
#include <boost/unordered_map.hpp>
#include "CommonsNative.h"

class InstrumentsStorage
{
public:
    static InstrumentsStorage& getInstance() {
        static InstrumentsStorage instance;
        return instance;
    }
    int GetInstrumentId(std::string& instrument);
    std::string& GetClassCode(int instrumentId) {
        return classcodes[instrumentId];
    }
    std::string& GetTicker(int instrumentId) {
        return tickers[instrumentId];
    }
private:
    InstrumentsStorage();
    InstrumentsStorage(InstrumentsStorage const&);
    void operator=(InstrumentsStorage const&);

    boost::unordered_map<std::string, int> instrument2id;
    std::string classcodes[MAX_INSTRUMENTS_NUMBER_IN_SYSTEM];
    std::string tickers[MAX_INSTRUMENTS_NUMBER_IN_SYSTEM];
};

cpp:

#include "InstrumentsStorage.h"
#include <boost/property_tree/ptree.hpp>
#include <boost/property_tree/ini_parser.hpp>
#include <iostream>

InstrumentsStorage::InstrumentsStorage() {
    std::cout << "InstrumentsStorage constructor called!" << std::endl;
    boost::property_tree::ptree pt;
    boost::property_tree::ini_parser::read_ini("config_generated/instruments_gate_0.txt", pt);

    for (auto& section : pt)
    {
        std::string instrument = section.first;
        int id =  section.second.get_value<int>();
        instrument2id[instrument] = id;
        std::cout << "InstrumentsStorage Assigned instrument = " << instrument << " id = " << id << std::endl;
        classcodes[id] = instrument.substr(0, 4);
        tickers[id] = instrument.substr(4, std::string::npos);
        std::cout << "InstrumentsStorage id " << id << " classcode = " << classcodes[id]
                << " ticker = " << tickers[id] << std::endl;
    }
}

int InstrumentsStorage::GetInstrumentId(std::string& instrument)
{
    // std::cout << "InstrumentsStorage GetInstrumentId called, instrument = " << instrument << std::endl;
    boost::unordered_map<std::string, int>::iterator i = instrument2id.find(instrument);
    if (i == instrument2id.end())
    {
        // std::cout << "InstrumentsStorage GetInstrumentId not found, result == -1 " << std::endl;
        return -1;
    } else
    {
        // std::cout << "InstrumentsStorage GetInstrumentId found, result = " << i->second << std::endl;
        return i->second;
    }
}

Like `InstrumentsStorage& is = InstrumentsStorage::getInstance();`? With the `&`? — Mark Garcia, Feb 12 '14 at 07:37
like that `int instrumentId = InstrumentsStorage::getInstance().GetInstrumentId(instrumentString);` and like that `std::string& ticker = InstrumentsStorage::getInstance().GetTicker(oa.InstrumentId);` — Oleg Vazhnev, Feb 12 '14 at 07:42
@juanchopanza i'm using c++11 features so I think so. i'm using Visual Studio 2012. — Oleg Vazhnev, Feb 12 '14 at 07:44
Could it be that you are creating an instance in one of your member functions? — UldisK, Feb 12 '14 at 07:44
this singleton is not thread safe, that is why you get two instances — BЈовић, Feb 12 '14 at 07:55
@BЈовић It is thread safe in C++11. It could be that the compiler doesn't implement this correctly. — juanchopanza, Feb 12 '14 at 07:56
@juanchopanza Cool, [you are right](http://stackoverflow.com/a/11711991/476681). — BЈовић, Feb 12 '14 at 08:03

Michael Foukarakis · Answer 1 · 2014-02-12T08:07:21.720

0

If you want a thread-safe singleton, you must implement your own locking mechanism. The simplest way to do so in C++11 is this:

#include <mutex>

class Singleton 
{
    static Singleton *singletonInstance;
    Singleton() {}
    static std::mutex m_;

  public:

    static Singleton* getSingletonInstance()
    {
        std::lock_guard<std::mutex> lock(m_);
        if(singletonInstance == nullptr)
        {
            singletonInstance = new Singleton();
        }
        return singletonInstance;
    }
}

Another way to implement it, perhaps simpler, is using a once function:

#include <mutex>

static void init_singleton() {
    singletonInstance = new Singleton;
}

static std::once_flag singleton_flag;

Singleton* getSingletonInstance() {
    std::call_once(singleton_flag, init_singleton);
    return singletonInstance;
}

Both the above work even if your compiler may not fully implement the C++11 concurrency model yet, where typing:

static Foo& getSingleton() { 
    static Foo foo;
    return foo;
}

will just work.

edited Feb 12 '14 at 08:07

answered Feb 12 '14 at 07:56

Michael Foukarakis

35,789
5
74
113

Ah yes, I will remove my previous comment. – juanchopanza Feb 12 '14 at 08:00
`static Foo foo{};` doesn't compile in Visual Studio 2012 – Oleg Vazhnev Feb 12 '14 at 08:04
so is it VS2012 bug? if upgrading to VS2013 will fix the problem? where this feature in this table http://msdn.microsoft.com/en-us/library/hh567368.aspx ? – Oleg Vazhnev Feb 12 '14 at 08:08
@javapowered: It is under "Magic statics", and it is indeed not supported yet (VS 2013). – Michael Foukarakis Feb 12 '14 at 08:09
if `std::call_once` is lock-free once singleton is initialized? for low-latency code it's better to have some kind of lock-free code, probably double-check locking – Oleg Vazhnev Feb 12 '14 at 08:20
Double-check locking is ridiculously hard to get right, with many opportunities for error. I'd rather it went away, and I will never suggest it to anyone. – Michael Foukarakis Feb 12 '14 at 08:23
i've found in logs that second `InstrumentsStorage constructor called!` line goes after two minutes(!) after first line. So in my case "multithreading" is not the problem. – Oleg Vazhnev Feb 12 '14 at 08:29
Are you sure your singleton is never destroyed? Using a debugger, can you set a breakpoint in the singleton's constructor and find out exactly where it is called? That should give you a clue what's going on. – Michael Foukarakis Feb 12 '14 at 08:48
even if it destroyed, static field should not be reset, isn't it? i've added tracing - destructor is not called. – Oleg Vazhnev Feb 12 '14 at 11:06
i now think that actually I have two different process, third-party library just creates one extra process for me. I just need to print pid every time singleton created but I don't know how to do that in Windows, in linux http://man7.org/linux/man-pages/man2/getpid.2.html – Oleg Vazhnev Feb 12 '14 at 13:15

score 0 · Answer 2 · answered Feb 12 '14 at 08:33

Am not sure if your problem is resolved. Looking at code, it seems the static variable should be instantiated only once so constructor for it should be only called once. May be the constructor is called some other time in your code elsewhere. Can you set a breakpoint at constructor and debug it to check. It may help you find the issue faster.

why my implementation of singleton instatiated twice? (one process, several threads)

2 Answers2