threadsicheres singleton

[XSpille]

Die Variante bei wikipedia ist etwas inperformant, da sie bei jedem
getInstance einen Lock braucht.

Man kann den Lock aber auch in den if-Teil verlagern und dort einen Lock
machen. Jedoch benötigt man unter dem Lock wieder die if-Abfrage.

Der Vorteil ist, dass der relativ kostspielige Mutex nur zu Beginn verwendet
wird. Wenn der Zeiger einmal auf ein Objekt verweist, jedoch nicht mehr.

Gruß,
XSpille

[mal_so_am_rande]

Natürlich muss man immer bei der Nutzung eines Objekts von mehreren Threads aus aufpassen -- ob Singleton oder nicht. Dazu haben wir Dinge, wie Mutexes und Locks. Darüber hinaus muss man darauf achten, dass es bei der Erzeugung des Singleton-Objekts zu keinen Datenrennen kommt. Ich weiß nicht, wie es die anderen Compiler-Hersteller handhaben, aber der GCC synchronisiert Dinge wie folgendes:

class Foo {...};

Foo& Foo::getInstance() // statische Elementfunktion
{
  static Foo f;
  return f;
{

von ganz alleine. Wenn also die Funktion noch nie aufgerufen wurde und zwei Threads rufen sie gleichzeitig auf, dann geht das beim GCC nicht schief -- es sei denn, man hat diese Synchronisierung per Compiler-Option ausgeschaltet. Der aktuelle C++ Standard macht, was das angeht, keine Garantien. Aber der kommende Standard wird so etwas erlauben. Guck doch mal einfach in der Doku Deines Compilers nach. Früher oder soäter sollte jeder C++ Compiler, der etwas auf sich hält, entsprechenden Maschinencode einfügen, um Datenrennen bei der Initialisierung von solchen Objekten zu verhindern.

[hustbaer]

@mal_so_am_rande:
Mit MSVC 6 bis 10 ist der von dir gepostete Code *nicht* thread-safe. (Mit MSVC 10 vermutlich auch nicht, müsste ich aber erst nochmal checken um 100% sicher zu sein.)

[hustbaer]

XSpille schrieb:

Die Variante bei wikipedia ist etwas inperformant, da sie bei jedem
getInstance einen Lock braucht.

Dafür funktioniert sie

Man kann den Lock aber auch in den if-Teil verlagern und dort einen Lock
machen. Jedoch benötigt man unter dem Lock wieder die if-Abfrage.

Der Vorteil ist, dass der relativ kostspielige Mutex nur zu Beginn verwendet
wird. Wenn der Zeiger einmal auf ein Objekt verweist, jedoch nicht mehr.

Der Nachteil ist, dass es so-ohne-weiteres nicht funktioniert.

Nennt sich Double-Checked-Locking, und wurde zur Genüge in diversen Blogs/Artikeln durchgekaut. Fazit: es geht, aber nur wenn man's richtig macht, und nur wenn man bestimmte Funktionen zur Verfügung hat bzw. sich auf nicht vom Standard vorgeschriebene Garantien bestimmter Compiler verlässt.

http://en.wikipedia.org/wiki/Double-checked_locking

[XSpille]

hustbaer schrieb:

Der Nachteil ist, dass es so-ohne-weiteres nicht funktioniert.

Ich lerne gerne dazu...

Was funktioniert an dem folgenden Code (ungetestet -> Syntaxfehler ignorieren) nicht?

Foo* Foo::instance = 0;

Foo& Foo::getInstance(){
	if(!instance){
		boost::mutex::scoped_lock l(mutex); 
		if(!instance){
			Foo::instance = new Foo();
		}
	}
	return *Foo::instance;
}

Einfach ein zusätzliches if um den generellen Lock zu vermeiden...

EDIT:

hustbaer schrieb:

und nur wenn man bestimmte Funktionen zur Verfügung hat bzw. sich auf nicht vom Standard vorgeschriebene Garantien bestimmter Compiler verlässt.

Worauf verlass ich mich, das der Standard mir nicht garantiert?
Oder meinst du die nicht im Standard enthaltenen Funktionen, die boost verwendet?

Gruß,
XSpille

[Shade Of Mine]

Das Problem ist, dass die Zuweisung Foo::instance = new Foo(); nicht atomic ist.

Sprich: instance koennte einen Wert bekommen bevor Foo komplett konstruiert ist. Wenn das der Fall ist und ploetzlich ein anderer Thread in die Funktion kommt, dann sieht dieser Thread ploetzlich: oh, instance hat einen wert, und returned ihn sofort - bevor Foo komplett konstruiert wurde.

Die einfachste Loesung ist ein Compare and Set (CAS) zu verwenden:

Foo& Foo::getInstance(){
    if(!instance){
        boost::mutex::scoped_lock l(mutex); 
        if(!instance){
            Foo* temp = new Foo();
            InterlockedExchange(&Foo::instance, temp);
            //syntax may vary
        }
    }
    return *Foo::instance;
}

uU hilft auch ein volatile, manche compiler garantieren dann diese wie eine Barriere zu behandeln - aber ich denke dass das kein Standardverhalten ist.

[XSpille]

Danke für die Antwort!

Shade Of Mine schrieb:

Sprich: instance koennte einen Wert bekommen bevor Foo komplett konstruiert ist.

Wozu verwendest du denn den InterlockedExchange ?
Ist in der Zeile danach etwa auch noch nicht garantiert, dass das Objekt
komplett konstruiert ist oder ist eine Zeigerzuweisung auch nicht
zwangsläufig atomar?

EDIT:
Ich poste einfach noch mal diesen schon häufiger verlinkten multithreaded Singleton
Artikel, den ich in meinem persönlichen wiki verlinkt hatte:
http://uint32t.blogspot.com/2007/12/you-lazy-bastard-part-1.html

[Shade Of Mine]

XSpille schrieb:

Ist in der Zeile danach etwa auch noch nicht garantiert, dass das Objekt
komplett konstruiert ist oder ist eine Zeigerzuweisung auch nicht
zwangsläufig atomar?

Die Zuweisung ist nicht atomar. Auch wenn die wahrscheinlichkeit sehr sehr gering ist, dass der Thread gerade waehrend der enorm kurzen Zuweisung unterbrochen wird - die Gefahr besteht immer.

Deshalb verwende ich hier eine atomic Zuweisung. In echtem Code wuerde ich instance zu einem atomic<Foo*> machen. Wobei atomic eine Klasse ist, die mir eben atomic Operations bietet. Java hat da zB AtomicReference.

Wobei der Singleton in Java natuerlich viel einfacher geht indem man statische Initialisierung nutzt und somit das Problem des lockings der VM uebergibt.

Ich poste einfach noch mal diesen schon häufiger verlinkten multithreaded Singleton
Artikel, den ich in meinem persönlichen wiki verlinkt hatte:
http://uint32t.blogspot.com/2007/12/you-lazy-bastard-part-1.html

Moeglich dass er funktioniert. Haengt davon ab was call_once genau macht. Ich habe es nie verwendet, deshalb keine Ahnung.

Ich wuerde dennoch korrektes double checked locking verwenden - eben mit einem atomic Zeiger. Denn der Vorteil davon ist eben, dass sobald der Singleton steht, die kosten fuer getInstance minimal sind. Man kann hier ja auch ein static Objekt nehmen indem man statt new Foo() ein

Foo* helper() {
  static Foo instance;
  return &instance;
}
...
Foo* temp = helper();

macht...

[hustbaer]

Shade Of Mine schrieb:

Die einfachste Loesung ist ein Compare and Set (CAS) zu verwenden:

Foo& Foo::getInstance(){
    if(!instance){
        boost::mutex::scoped_lock l(mutex); 
        if(!instance){
            Foo* temp = new Foo();
            InterlockedExchange(&Foo::instance, temp);
            //syntax may vary
        }
    }
    return *Foo::instance;
}

Das ist schon näher dran, reicht aber immer noch nicht.

Ein Thread könnte instance != 0 sehen, aber andere Änderungen (z.B. den eigentlichen Inhalt des Singletons) noch nicht mitbekommen haben.

Genau genommen müsste da nichtmal die CPU/Caches dazwischenfunken, der Compiler selbst könnte hier bereits Reordering machen welches erlaubt wäre, und ausreichend um beliebig schlimme Fehler zu produzieren.

Du musst das if (!instance) noch irgendwie mit acquire Semantik ausstatten.
Dazu kann man z.B. wieder CAS verwenden (z.B. InterlockedCompareExchange(&var, 0, 0)), oder aber - wenn man z.B. MSVC verwendet - volatile .

Shade Of Mine schrieb:

Ich poste einfach noch mal diesen schon häufiger verlinkten multithreaded Singleton
Artikel, den ich in meinem persönlichen wiki verlinkt hatte:
http://uint32t.blogspot.com/2007/12/you-lazy-bastard-part-1.html

Moeglich dass er funktioniert. Haengt davon ab was call_once genau macht. Ich habe es nie verwendet, deshalb keine Ahnung.

Es macht das was dein Code machen möchte, nur richtig.

[Shade Of Mine]

hustbaer schrieb:

Das ist schon näher dran, reicht aber immer noch nicht.

Stimmt.
Wie gesagt: bei mir waere instance eine atomic Klasse und daher waere es korrekt.

Aber das ist wiedermal ein super Beispiel: sicherer multithreading code ist enorm schwer

[hustbaer]

Shade Of Mine schrieb:

Aber das ist wiedermal ein super Beispiel: sicherer multithreading code ist enorm schwer

Tjoa...
Hauptsächlich wenn man versucht schlau zu sein, und versucht etwas zu optimieren, was man nicht ausreichend verstanden hat

Wobei das wohl auf die meisten Bereiche zutrifft, nicht nur Multithreading.
(Allerdings kommt mir vor, dass beim Thema Multithreading der "glaubt-verstanden-zu-haben-aber-hat-nicht-verstanden" Anteil unter den Programmierern viel höher ist, als wenn es um andere Themen geht.)

[vbnm]

mal_so_am_rande schrieb:

aber der GCC synchronisiert Dinge wie folgendes:

class Foo {...};

Foo& Foo::getInstance() // statische Elementfunktion
{
  static Foo f;
  return f;
{

von ganz alleine.

gut ich nutze den gcc und habe momentan nicht vor das ganze weiterzugeben.

dann noch verständnissfragen:
1.
wäre das threadsicher?

class Foo {static Foo f;};
Foo& Foo::getInstance(){return f;}

sind atomic operationen immer threadsicher?

wenn ja:
3.
ist das verändern/zuweisen eines bools/unsigned ints atomic?

ist das schreiben in einen std::ofstream atomic oder von hause aus threadsicher?

gibt es eine liste von atomic operationen?

[Gast]

Nein, auch ein static in der Klasse ist nicht zwingend threadsicher. Das steht - besser erklärt als ich es könnte - ebenfalls in dem verlinkten Artikel drin.

Sofern man keine atomics zur Verfügung hat, halte ich zwei Ansätze für sinnvoll:

1. Die bevorzugte Lösung ist, einfaches Locking zu verwenden, sofern das kein Performanceproblem ist - was ich immer erst mal durch eine Messung belege
2. Beim Applikationsstart - wenn alles noch single-threaded ist - die Singletons einmal abfragen. Dann ist die ganze Locking-Problematik obsolet, weil danach alle Pointer/Referenzen der Singletons initialisiert sind. Kritischer Punkt ist, dass eine zentrale Initialisierung alle Singletons kennen muss.

Der zweite Ansatz ist wenig elegant. Er ist aber immer noch besser, als dass einem im Betrieb die Applikation wegen einer Race-Condition um die Ohren fliegt.

[krümelkacker]

mal_so_am_rande schrieb:

[...] aber der GCC synchronisiert Dinge wie folgendes:

class Foo {...};

Foo& Foo::getInstance() // statische Elementfunktion
{
  static Foo f;
  return f;
{

von ganz alleine.

...per default. Dies lässt sich deaktivieren:

GCC 4.5.1 Dokumentation, Command Options, C++ Dialect Options schrieb:

-fno-threadsafe-statics
Do not emit the extra code to use the routines specified in the C++ ABI for thread-safe initialization of local statics. You can use this option to reduce code size slightly in code that doesn't need to be thread-safe.

mal_so_am_rande schrieb:

Früher oder soäter sollte jeder C++ Compiler, der etwas auf sich hält, entsprechenden Maschinencode einfügen, um Datenrennen bei der Initialisierung von solchen Objekten zu verhindern.

Wenn interessiert, wo das genau steht, ich habe das mal rausgesucht:

N3126.pdf, 6.7/4 schrieb:

[...] Otherwise such a variable is initialized the first time control passes through its declaration; such a variable is considered initialized upon the completion of its initialization. [...] If control enters the declaration concurrently while the variable is being initialized, the concurrent execution shall wait for completion of the initialization. [...]

Das gilt wohlgemerkt nur für C++0x und ist beim aktuellen GCC als "Language Extension" zu verbuchen. Wenn man auf Nummer sicher gehen will (und es auch bei anderen Compilern funzen soll), muss man sich in C++03 selbst um die Synchronisierung kümmern.

kk

[hustbaer]

ogni42 schrieb:

2. Beim Applikationsstart - wenn alles noch single-threaded ist

Das ist eine ... "mutige" Annahme.

[Gast]

Nein, das ist keine Annahme, sondern eine Voraussetzung Die auf Gültigkeit zu prüfen, ist Aufgabe des Programmierers.

[hustbaer]

Ich kenne ein paar Projekte wo diese Voraussetzung nicht erfüllt wird.
Und ich ärgere mich immer wieder, wenn irgend eine Library irgendwelche #@§%!!~ Voraussetzung mitbringt, nur weil der Programmierer zu faul war ein bestimmtes Problem ordentlich zu lösen.

[gastgast]

OKOKOKOK Ihr habt mich überzeugt. War eine Sch...idee

[vbnm]

ok war einige zeit nicht da und kann leider jetzt erst wieder antworten.

kann man nich statt dem in dem link geschickten code auch das schreiben?
(habe mal das template und den instance_helper gelöscht)

#include <boost/thread/once.hpp>
struct singleton : private boost::noncopyable{
public:
 static singleton  instance(){
   boost::call_once(call_me_once,once_);
   static singleton  s;
   return s;
 }   

private:
 static boost::once_flag once_;
 static void call_me_once(){
   instance();
 }

 singleton();
 ~singleton(); 
};
boost::once_flag singleton::once_ = BOOST_ONCE_INIT;

[hustbaer]

Das wird vermutlich in einem Stack-Overflow enden. Also der instance_helper macht schon Sinn, bzw. genauer: er ist nötig.

Und natürlich kannst du das Singleton nicht "by value" zurückgeben, da es ja noncopyable ist. Ausserdem wäre es - selbst wenn es kopierbar wäre - dann kein Singleton mehr. Den Return-Typ von "instance" solltest du also auch wieder zu "singleton&" zurück-ändern.

ps: das Entfernen des Templates ist natürlich OK. Auch wenn dadurch der Typ des Singletons (mMn unnötigerweise) festgelegt wird, und die schöne Wiederverwendbarkeit dadurch flöten geht.