3. Portables Multithreaded Singleton

Portables Multithreaded Singleton

  • X

    Hallo zusammen,

    ich habe hier (glaube ich) ein portables multithreaded Singleton.
    Eigentlich bin ich davon überzeugt, dass es threadsicher ist.
    Könnt ihr mir das bestätigen oder fällt euch doch noch ein Fehler auf?

    #include<iostream>
#include<atomic>
#include<boost/thread/mutex.hpp>

class Foo{

public:
        static Foo& getInstance();

private:
        static std::atomic<Foo*> singleton;
        static boost::mutex singletonMutex;
};

std::atomic<Foo*> Foo::singleton(0);
boost::mutex Foo::singletonMutex;

Foo& Foo::getInstance(){
        Foo* foo = singleton.load(); // keinen gecachten Wert! (load ist volatile)
        if(!foo){ // Wenn kein Singleton existiert -> erstellen
                boost::mutex::scoped_lock l(singletonMutex); // Hier darf nur ein Thread rein
                foo = singleton.load(); // keinen gecachten Wert! (load ist volatile)
                if(!foo){ // falls im if doch 2 Threads sind (möglich!)
                        foo = new Foo();
                        singleton.exchange(foo); // atomare Operation & volatile verhindert Reihenfolgevertauschungen -> foo vollständig erstellt
                }
        }
        return *foo;
}

    oder wie schaut es mit dieser Alternative aus:

    class Foo{

public:
        static Foo& getInstance();
        static void initInstance();

private:
        static std::atomic<Foo*> singleton;
        static boost::once_flag once;
};

std::atomic<Foo*> Foo::singleton(0);
boost::once_flag Foo::once = BOOST_ONCE_INIT;

void Foo::initInstance(){
        Foo* temp = new Foo();
        singleton.exchange(temp); // atomare Operation & volatile verhindert Reihenfolgevertauschungen -> temp vollständig erstellt
}

Foo& Foo::getInstance(){
        Foo* foo = singleton.load(); // keinen gecachten Wert! (load ist volatile)
        if(!foo){ // Wenn kein Singleton existiert -> erstellen
                boost::call_once(initInstance, once); // Nur einmal initialisieren
                return *singleton.load(); // hole ungecachten Wert
        }
        return *foo;
}

    Gibt es irgendwelche Aspekte, die ich nicht berücksichtigt habe?

    Gruß,
    XSpille

    0
  • ?

    Gut, dass du load und exchange nicht zeigst. 🙄

    0
  • X

    ooooooooooooooooooooooooo schrieb:

    Gut, dass du load und exchange nicht zeigst. 🙄

    Kann ich ja schlecht, wenn es Teil der Standard-Template-Library ist (C++0x) 😮

    Aber ich kann gerne einen Link zur API zur Verfügung stellen: http://gcc.gnu.org/onlinedocs/libstdc++/latest-doxygen/a00371.html
    Dieser enthält auch die gcc-Sourcen 😉

    0
  • X

    Was mir gerade auffällt:

    Muss

    static std::atomic<Foo*> singleton;

    auch volatile sein?

    static volatile std::atomic<Foo*> singleton;

    EDIT: Je länger ich drüber nachdenke, glaube ich jedoch nicht...

    Leider kompiliert er dann nicht mehr:

    singleton.cpp:20:28: error: passing 'volatile std::atomic<Foo*>' as 'this' argument of '_Tp* std::atomic<_Tp*>::load(std::memory_order) const [with _Tp = Foo, std::memory_order = std::memory_order]' discards qualifiers
singleton.cpp:23:24: error: passing 'volatile std::atomic<Foo*>' as 'this' argument of '_Tp* std::atomic<_Tp*>::load(std::memory_order) const [with _Tp = Foo, std::memory_order = std::memory_order]' discards qualifiers
singleton.cpp:27:26: error: passing 'volatile std::atomic<Foo*>' as 'this' argument of '_Tp* std::atomic<_Tp*>::exchange(_Tp*, std::memory_order) [with _Tp = Foo, std::memory_order = std::memory_order]' discards qualifiers

    Versteh ich eigentlich nicht, denn die Funktionen sind ja volatile:
    http://gcc.gnu.org/onlinedocs/libstdc++/latest-doxygen/a00371.html

    0
  • S

    Nein, du brauchst kein volatile. Das macht das atomic Objekt intern. Immer wenn du load() aufrufst wird der Wert direkt aus dem Speicher neu gelesen und alles caching umgangen.

    PS: deine 1. Variante sieht gut aus. zur 2. sage ich nichts, da ich call_once noch nie verwendet habe.

    0
  • X

    Shade Of Mine schrieb:

    Nein, du brauchst kein volatile. Das macht das atomic Objekt intern. Immer wenn du load() aufrufst wird der Wert direkt aus dem Speicher neu gelesen und alles caching umgangen.

    THX

    0
  • X

    Aus Gründen der Vollständigkeit hier meine Singleton-Versionen noch als
    Template.

    Variante 1:

    #include<iostream>
#include<atomic>
#include<boost/thread/mutex.hpp>
#include<boost/noncopyable.hpp>

template<typename T>
class Singleton : private boost::noncopyable{

public:
        static T& getInstance(){
                T* instance = singleton.load();
                if(!instance){
                        boost::mutex::scoped_lock l(singletonMutex);
                        instance = singleton.load();
                        if(!instance){
                                instance = createInstance();
                                singleton.exchange(instance);
                        }
                }
                return *instance;
        }

private:
        static T* createInstance(){
                return new T();
        }

        static std::atomic<T*> singleton;
        static boost::mutex singletonMutex;
};

template<typename T>
std::atomic<T*> Singleton<T>::singleton(0);
template<typename T>
boost::mutex Singleton<T>::singletonMutex;

    Variante 2:

    #include<iostream>
#include<atomic>
#include<boost/thread/once.hpp>
#include<boost/noncopyable.hpp>

template<typename T>
class Singleton : private boost::noncopyable{

public:
        static T& getInstance(){
                T* instance = singleton.load();
                if(!instance){
                        boost::call_once(initInstance, once);
                        return *singleton.load();
                }
                return *instance;
        }

private:
        static void initInstance(){
                T* instance = createInstance();
                singleton.exchange(instance);
        }

        static T* createInstance(){
                return new T();
        }

        static std::atomic<T*> singleton;
        static boost::once_flag once;
};

template<typename T>
std::atomic<T*> Singleton<T>::singleton(0);
template<typename T>
boost::once_flag Singleton<T>::once = BOOST_ONCE_INIT;

    Die Instanz-Erzeugung habe ich ausgegliedert, damit für bestimmte Typen
    Template-Spezialisierungen erstellt werden können.

    class FooBar : public Singleton<FooBar>{
public:
        FooBar(int i){
        }

};

template<>
FooBar* Singleton<FooBar>::createInstance(){
        return new FooBar(1);
}

    Gruß,
    XSpille

    0
  • ?

    XSpille schrieb:

    Hallo zusammen,

    ich habe hier (glaube ich) ein portables multithreaded Singleton.
    Eigentlich bin ich davon überzeugt, dass es threadsicher ist.
    Könnt ihr mir das bestätigen oder fällt euch doch noch ein Fehler auf?

    #include<iostream>
#include<atomic>
#include<boost/thread/mutex.hpp>

class Foo{

public:
        static Foo& getInstance();

private:
        static std::atomic<Foo*> singleton;
        static boost::mutex singletonMutex;
};

std::atomic<Foo*> Foo::singleton(0);
boost::mutex Foo::singletonMutex;

Foo& Foo::getInstance(){
        Foo* foo = singleton.load(); // keinen gecachten Wert! (load ist volatile)
        if(!foo){ // Wenn kein Singleton existiert -> erstellen
                boost::mutex::scoped_lock l(singletonMutex); // Hier darf nur ein Thread rein
                foo = singleton.load(); // keinen gecachten Wert! (load ist volatile)
                if(!foo){ // falls im if doch 2 Threads sind (möglich!)
                        foo = new Foo();
                        singleton.exchange(foo); // atomare Operation & volatile verhindert Reihenfolgevertauschungen -> foo vollständig erstellt
                }
        }
        return *foo;
}

    Foo& Foo::getInstance(){
        boost::mutex::scoped_lock l(singletonMutex); // Hier darf nur ein Thread rein
        foo = singleton.load(); // keinen gecachten Wert! (load ist volatile)
        if(!foo){ 
                foo = new Foo();
                singleton.exchange(foo); // atomare Operation & volatile verhindert Reihenfolgevertauschungen -> foo vollständig erstellt
        }
        return *foo;
}

    Würde das nicht auch reichen oder holst du dein Singleton 1000 mal pro Sekunde?

    0
  • X

    x8x7x6x5x4 schrieb:

    Foo& Foo::getInstance(){
        boost::mutex::scoped_lock l(singletonMutex); // Hier darf nur ein Thread rein
        Foo* foo = singleton.load(); // keinen gecachten Wert! (load ist volatile)
        if(!foo){ 
                foo = new Foo();
                singleton.exchange(foo); // atomare Operation & volatile verhindert Reihenfolgevertauschungen -> foo vollständig erstellt
        }
        return *foo;
}

    Würde das nicht auch reichen oder holst du dein Singleton 1000 mal pro Sekunde?

    Du hast recht, das äußere if ist nur wegen Performance-Gründen.
    Deine gepostete Kurzform ist (soweit ich es beurteilen kann) ebenfalls thread-sicher.

    EDIT: Ehrlich gesagt hab ich überlegt, ob ich um dieses äußere if noch
    ein weiteres if ergänzen kann, bei dem ich den gecachten Wert auslese. 🙄
    (Wenn einmal etwas drinsteht, wird es nicht mehr verändert)
    Aber ich glaube da kommt man mit atomic nicht dran.
    Generell wollte ich durch dieses Singleton den Synchronisation-Lock vermeiden.

    0

  • @x8x7x6x5x4:
    Gerade durch die "kann ich mir ohne Kontext von überall holen" Eigenschaft von Singletons, werden sie in vielen Programmen wirklich sehr oft geholt.
    Daher macht das double-checked-locking durchaus viel Sinn.
    Ein "load-acquire" ist auf vielen Plattformen um mehrere Grössenordnungen billiger als ein "interlocked-compare-exchange" (aka. compare-and-swap).
    (Und ne Mutex locken braucht - zumindest - ein "interlocked-compare-exchange")

    @XSpille:
    boost::call_once macht - zumindest in der Windows Implementierung - intern schon selbst double checked locking. Es ausserhalb nochmal zu machen bringt dann nicht viel. Wobei ich gestehen muss, dass ich es auch so wie du schreiben würde wenn mir die Performance an der Stelle wirklich wichtig ist.

    0
  • C
    Mod

    Man könnte noch die Anforderungen an Memory-Ordering etwas abschwächen

    #include <atomic>
#include <mutex>

class Foo
{
public:
    static Foo& getInstance();
private:
    static std::atomic<Foo*> singleton;
    static std::mutex singletonMutex;
};

std::atomic<Foo*> Foo::singleton = nullptr;
std::mutex Foo::singletonMutex;

Foo& Foo::getInstance()
{
// Ordnungsmatrix
// A(erzeugender thread, 1. Aufruf)->B(erzeugender thread, 1. Aufruf)
//     B   (0)           (1)           (2)                           Ergebnis           Begründung
// A
// (0)      x            seq bef       seq bef                       : nullptr (init)
// (1)      seq aft      x             seq bef                       : nullptr (init)
// (2)      seq aft      seq aft       x                             := Foo
// A(erzeugender thread, 2. Aufruf)->B(erzeugender thread, 1. Aufruf)
//     B   (0)           (1)           (2)
// A
// (0)      seq aft      seq aft       seq aft                       : Foo              (0) seq aft (2)

// A(anderer thread, 1.Aufruf)->B(erzeugender thread, 1.Aufruf)
//     B   (0)           (1)           (2)
// A
// (0)     ind           ind           ind                           : nullptr od. Foo
// (1)     ind           ind           inter-thread happens after    : Foo              (sync release->aquire)
// A(anderer thread, 2.Aufruf)->B(anderer thread, 1.Aufruf)
//     B   (0)           (1)
// A
// (0)     seq aft       seq aft                                     : Foo              (0) seq aft (1)
    Foo* foo = singleton.load(std::memory_order_relaxed);    // (0)
    if(foo==nullptr)
    {
        std::lock_guard<std::mutex> lock(singletonMutex);
        foo = singleton.load(std::memory_order_aquire);      // (1)
        if(foo==nullptr)
        {
            foo = new Foo();
            singleton.store(foo, std::memory_order_release); // (2)
        }
    }
    return *foo;
}

    mit tls geht es auch ganz ohne atomics (der mutex genügt für die Synchronisation)

    #include <mutex>

class Foo
{
public:
    static Foo& getInstance();
private:
    Foo* singleton;
    static std::mutex singletonMutex;
};

Foo* Foo::singleton = nullptr;
std::mutex Foo::singletonMutex;

Foo& Foo::getInstance()
{
    static thread_local Foo* p = nullptr;
//  return *(p?p:(std::lock_guard<std::mutex>(singletonMutex),p=(singleton?singleton:singleton=new Foo())));
    if(p==nullptr)
    {
        std::lock_guard<std::mutex> lock(singletonMutex);
        if(singleton==nullptr)
        {
            singleton = new Foo();
        }
        p = singleton;
    }
    return *p;
}

    je nach Hardware ist das aber ggf. sogar langsamer.
    Alles völlig ungetested.

    0
  • ?

    Hat das jetzt ein Jahr gebraucht eine Lösung zu finden oder warum grabt ihr den Thread wieder aus?

    0
  • C
    Mod

    datumsgrenze schrieb:

    Hat das jetzt ein Jahr gebraucht eine Lösung zu finden oder warum grabt ihr den Thread wieder aus?

    Warum fragst du?

    0

  • Der Thread wurde kürzlich verlinkt. Da die Antwort hier her gehört, und nicht in den anderen Thread, hab' ich sie auch hier rein geschrieben. Und den Thread damit wieder vorgeholt.

    0
Anmelden zum Antworten