Kiedyś na blogu opisywałem kilka implementacji singleton’a przystosowanych do pracy w środowisku wielowątkowym. Jedna z nich opierała się na tzw. double checked locking:
public sealed class Singleton { private static Singleton m_Instance = null; private static readonly object m_Sync = new object(); private Singleton() { } public static Singleton Instance { get { if(m_Instance == null ) { lock(m_Sync) { if(m_Instance == null) m_Instance = new Signleton(); } } return m_Instance; } } }
Niestety implementacja nie jest w pełni poprawna. Brakuje w niej modyfikatora volatile na polu m_Instance. Jeśli nie jest dla Was zrozumiałe co robi słówko volatile zachęcam do przeczytania mojego wpisu o tym. Bez tego ciężko będzie zrozumieć dlaczego volatile jest konieczny. Poprawna implementacja wygląda więc następująco:
public sealed class Singleton { private static volatile Singleton m_Instance = null; private static readonly object m_Sync = new object(); private Singleton() { } public static Singleton Instance { get { if(m_Instance == null ) { lock(m_Sync) { if(m_Instance == null) m_Instance = new Signleton(); } } return m_Instance; } } }
Z wpisu o volatile wiemy, że rdzenie w zależności od zaimplementowanego modelu pamięci, mogą wykonywać cache na polach typu read. Pole m_Instance zalicza się również do tego przykładu. W przypadku gdy jeden wątek stworzy nową instancję m_Instance, drugi wątek nie koniecznie zauważy tą zmianę. Pierwszy IF ( if(m_instance == null) jest niebezpieczny ponieważ istnieje ryzyko, że rdzeń aktualnie ma tą wartość w cache i zawsze warunek może być spełniony – mimo, że już w drugim wątku instancja została utworzona. Jeśli warunek jest spełniony wtedy zostanie wywołany lock, który z kolei wykona flush i następny warunek ( if(m_Instance==null)) nie zostanie spełniony, jednak niepotrzebnie zostanie nałożona blokada.
Innymi słowy, volatile jest potrzebny wyłącznie dla pierwszego if’a aby nie było konieczności zakładania bez potrzeby blokady.
W następnych postach zajmiemy się tzw. MemoryBarrier, który ma podobne zastosowanie do volatile i wiąże się ściśle z architekturą procesora oraz dokonywanymi optymalizacjami.
A nie można prościej i wydajniej (bez locks)?
public sealed class Singleton
{
private static readonly Singleton instance = new Singleton();
//konstruktor statyczny, żeby kompilator nie oznaczył klasy jako beforefieldinit
static Singleton()
{
}
public static Singleton Instance { get { return instance; } }
}
Chodzi o Lazy Loading. Kilka implementacji podawalem w jednym z poprzednim postow i double checked locking byl jednym z nich. Mozna rowniez i bez zadnych lockow uzywajac nested class.
Warto zauważyć, że skoro używamy singletonu, to chcemy mieć tylko jeden obiekt.
W scenariuszu optymistycznym (prawie 100%) obiekt mamy stworzony i 1. if jest niespełniony. Tylko na początku i to w przypadku wielowątkowej inicjalizacji może się zdarzyć sytuacja, którą opisujesz. W celu osiagniecia lepszej wydajności w przypadku optymistycznym ja pozbyłbym się volatile.
W .Net’ie 4.0 mamy klasę Lazy, która enkapsuluje takie zachowanie i umożliwia ciekawą konfigurację.
Moim zdaniem rozwiązanie Piotrka dalej nie jest poprawne. Problem stanowi instrukja
m_Instance = new Signleton();
Spodziewał byś się, że kompilator wygeneruje kod który zaalokuje pamięć wywoła konstruktor a następnie przypisze referencję na zaalokowaną pamięć do zmiennej m_Instance. Ale wcale tak być nie musi. Kolejność może być trochę inna: alokacja pamięci, przypisanie wskaźnika wskazującego na zaalokowaną pamięć do m_Instance a następnie wywołanie konstruktora.
W środowisku jednowątkowym nie było by najmniejszego problemu jednak tutaj drugi wątek uzna, że m_Instance jest różne od null i zacznie go używać kiedy drugi wątek nie skończył jeszcze wywołania konstruktora.
Zamiast tego rozwiązania po drugim ifie powinieneś zrobić co następuje:
Singleton tmp = new Signleton();
Interlocked.Exchange(ref m_Instance, tmp);
Interlocked.Exchange jest atomowa więc wszystko gra.
@Krzysztof:
IMHO rozwiazanie jest poprawne – nie trzeba uzywac Interlocked. Jedynie co mozna ulepszyc to zamiast volatile lepiej uzyc bariery ktora opisze w nastepnym poscie tj:
instance = new Singleton();
Thread.MemoryBarrier();
return instance;
Po za tym jak masz volatile, pole jest nie cachowane oraz nieoptymalizowane(re-ordering). Nie widze zatem problemu?
@Piotr
IMHO w CLR wywołanie locka daje pełny memory fence – wszystkie zapisy zmiennych które masz przed barierą muszą skończyć się przed barierą i wszystkie odczyty za barierą muszą zacząć się za barierą. W naszym przypadku oznacza to że po locku m_Instance musi być ponownie odczytana (nie może być zcachowana w rejestrze).
W Javie faktycznie byłoby jak mówisz – odczytany byłby stan zcachowany w rejestrze i mieli byśmy problem.
Co do reordering w instrukcji
m_Instance = new Signleton();
nie jestem przekonany, że volatile zagwarantuje na 100% to, że wywołanie konstruktora nastąpi przed przekazaniem referencji a nie po. Więc użyłbym Interlocked.Exchange a z volatile zrezygnował.
Generalnie problem o którym dyskutujemy jest bardzo dobrze opisany w książce Jeffreya Richtera CLR via C# wydanie 3 strona 844.
@Krzysztof:
Jesli chodzi o lock i czy nalezy bariere w nim umieszczac to postaram sie poszukac wiecej informacji. Gdzies wyczytalem, ze w przypadku tego singleton’a nalezy ale nie chce na razie nic stwierdzac – postaram sie w ciagu nastepnych dni dowiedziec dokladnie i wtedy napsize to w formie post’a (jednak najpierw wprowadzenie do bariery bo nie jest to temat powszechny).
Co do Volatile. Zgodnie z definicja pole volatile nie ulega re-order. Dlaczego wiec korzystac z Interlocked? Nawet resharper sugeruje oznaczenie pola jako volatile w przypadku double locked checking,
@Piotr
Reasumując volatile w Twojej implementacji Singletona jest jedynie potrzebny aby zagwarantować prawidłową kolejność kroków (przepisanie referencji, wywołanie konstruktora) podczas wykonania instrukcji
m_Instance = new Signleton();
Opisany przez Ciebie problem z cachowaniem IMHO nie istnieje gdyż sam lock gwarantuje pełny memory fence.
Jednak takie podejście ma konsekwencje – wszystkie odczyty singletona przez cały czas życia domeny aplikacji będą odczytami volatile co ma wpływ na gorszą wydajność.
Dlatego dużo lepszym rozwiązaniem na zapewnienie prawidłowego wykonania instrukcji
m_Instance = new Signleton();
jest skorzystanie z
Singleton tmp = new Signleton();
Interlocked.Exchange(ref m_Instance, tmp);
Podejście to nie wymaga użycia volatile (jest szybsze) i jednocześnie jednoznacznie wyraża intencje autora kodu oraz to co w algorytmie jest problematyczne – nie cache procesora tylko atomowość operacji przypisania referencji do w pełni skonstruowanego obiektu.
Reasumując raz jeszcze przykład z volatile działa ale IMHO nie z powodu który opisałeś w tym poście, użycie Interlocked.Exchange jest lepsze ze względu na wydajność oraz jednoznaczne wskazanie problematycznego miejsca w algorytmie.
@Krzysztof:
Zgadza sie – volatile dziala ale nie jest najbardziej optymalny. Zapobiega on re-orderingowi itp.
A co do bariery w locku to jest potrzebna (nie chodzi akurat o flush ale o re-ordering):
http://blogs.msdn.com/b/brada/archive/2004/05/12/volatile-and-memorybarrier.aspx
Co do Interlocked.Exchange. Nie jestem pewny co do Twojej implementacji. Czy nie jest mozliwe nastepujace uporzadkowanie operacji:
1. Alokacja pamieci tmp.
2. Exchanged miedzy m_Instance a tmp.
3. Wywolanie wlasciwego konstruktora i wypelnienie pol.
Co o tym sadzisz?
Dostarczony przez Ciebie link tylko potwierdza to co napisałem, że problemem nie jest cacheowanie tylko kolejność operacji w instrukcji przypisania. A najlepszym sposobem na zapewnienie prawidłowej kolejności jest opisany przeze mnie sposób z Interlocked.Exchange (nie trzeba używać do tego celu bariery).
Nie, opisana przez Ciebie sytuacja w proponowanym przeze mnie rozwiązaniu nie ma prawa wystąpić. Powołuję się przy tym na wiarygodne źródło Jeffrey Richter jest wieloletnim konsultantem zespołu który pracuje nad .NET Framework. Przed dalszą rozmową proszę o przeczytanie rozdziału “The Famous Double-Check Locking Technique” (str. 844) z jego książki CLR via C# wydanie 3.
@Krzysztof:
Napisales, ze bariera nie jest potrzebna to podeslalem link ktory pokazuje, ze jednak jest potrzebna.
Jak pisalem problem jest z re-ordering + cache. Co do Cache to juz pisalem kiedys o tym na blogu. LOCK powoduke flush ale problem jest z pierwszym ifem. Cache – pierwszy if, reordering to co jest w lock. Zmiana na publicznym polu dokonana w jednym watku nie musi byc widziana od razu z innych watkow (cache). Link podeslalem jednak aby pokazac, ze bariera jest potrzebna a nie aby debatowac nad cache vs reordering.
OK ale jak autor tlumaczy ta konstrukcje? Dlaczego ona dziala?
Nie napisałem, że jakakolwiek bariera nie jest potrzebna tylko, że można zastąpić rozwiązanie znajdujące się w załączonym przez Ciebie linku:
Singleton newVal = new Singleton();
System.Threading.Thread.MemoryBarrier();
value = newVal;
rozwiązaniem krótszym IMHO bardziej oddającym intencje autora:
Singleton tmp = new Signleton();
Interlocked.Exchange(ref m_Instance, tmp);
Rozwiązanie jest poprawne gdyż Interlocked.Exchange gwarantuje atomowość operacji oraz to, że wprowadzone przy jego pomocy zmiany będą od razu widoczne w pozostałych wątkach. Jak to robi to już jest szczegół implementacyjny nad którym końcowy programista nie koniecznie musi się zastanawiać, zwłaszcza, że odpowiedź może być różna dla różnych docelowych procesorów. W większości przypadków można założyć, że Interlocked.Exchange tworzy memory barrier.
Co do porównania z volatile to się zgadzamy, że to rozwiązanie jest szybsze ze względu na wszystkie kolejne odczyty.