Could you hire me? Contact me if you like what I’ve done in this article and think I can create value for your company with my skills.

January 21, 2011 / by Zsolt Soczó

Csúnya multithreading hiba

Double checked lockingnak indult, de bug lett belőle. Mit rontottam el?

private static volatile Dictionary> holidayDays;
private static readonly object staticLock = new object();

private Dictionary> GetHolidayDays()
{
if (holidayDays == null)
{
lock (staticLock)
{
if (holidayDays == null)
{
holidayDays = new Dictionary>();
FillTradingHours(holidayDays, “HOL”);
}
}
}
return holidayDays;
}

Could you hire me? Contact me if you like what I’ve done in this article and think I can create value for your company with my skills.

LEAVE A COMMENT

8 COMMENTS

  • Tóth Viktor January 21, 2011

    hát mondjuk ha túl sokág fut a FillTradingHours()-od, és közben bejön egy másik szál, akkor ő azt látja, hogy a holidayDays nem null, szépen visszaadja a félig feldolgozott holidayDays-t és ez okozhat hibát. Szóval szerintem így kellene:

    var temp = new Dictionary
    FillTradingHours(temp, “HOL”);
    holidayDays = temp;

    egyébként szerintem nem kell volatile-re rakni a holidayDays-t, mivel a lock miatt úgyis flush-olja a cache-t és érvényteleníti, ha jól tudom. A CLR via C#-ban ez le van írva, csak most nincs nálam utánanézni. De így emlékszem.

  • Soczó Zsolt January 21, 2011

    Viktor: igen, ez a hiba. :)
    Egyszerűen nem lett több szálnak ellenálló, ami az eredeti célja volt.
    A volatile lockos helyzetben 2.0 óra már nem kötelező, de nem árt. Én a linkelt. asszem 3. cikkben olvastam róla (eredetileg nem is volt a kódomban).
    Ha van időd elolvasni érdekelne a koklúzió ez ügyben.

  • Tóth Viktor January 22, 2011

    Elolvastam a cikkeket, illetve utánanéztem a CLR via C#-ban is, és nekem ezek jöttek le:

    az első megadott linkednél ad egy assembly kódot a cikkíró, és azzal bizonygatja, hogy mégis kell a volatile, mert különben a jitter a lock ellenére még mindig optimalizálna, és nem olvasná újra a változót. Nekem ebben azonban van pár gyanús dolog.

    Egyrészt a lock kódja felkerül a függvény tetejére, kikerülve az első if-et. (“Does The “lock” Keyword Affect Memory Barriers?” szekció). Ez teljesen más szemantika, mint a double check lock, miért kellene a két kisárgított kódnál újraolvasni a referenciát ebben a felállásban? A lock egy úgynevezett full fence-t képez, ami annyit jelent, hogy minden a lock előtt leírt írásnak be kell fejeződni a lock előtt, és minden a lock után leírt olvasásnak a lock után kell kezdődnie. A példakód assembly-je ennek eleget tesz. A két sárga if-es kód azért használja a regisztereket, mert a regiszterek betöltése a lock után történt, azaz a “lock után leírt olvasás a lock után kezdődik” kitétel teljesül.

    Ha a lock a két if között lenne, akkor a második if nem használhatná az edx-et, mivel annak a betöltése a lock előtt történt volna (a 0001b-0001e), ezután hívódna a lock (ami most a 00016-ban van), tehát a 00027-ben levő cmp, ami az if-nek felel meg, a lock-ot megelőző olvasás eredményét használná, és sérülne a fence. Ezért ekkor a generált kód kb úgy nézne ki, hogy a mostani 00025-ös jne után megint beolvasná az edx-be az 00001b-00001e nál is látott kóddal a cArr-t.

    A lényeg, hogy rossz a példaprogram, amit használt, mert rosszul értelmezte, hogy mit csinál a lock. Az általad megadott harmadik link 7-es figurájánál egyébként pont ezt a szerkezetet használja, és ő meg azt taglalja, hogy miért működik.

    Ezzel majdnem meg is nyugodhatnánk, azonban a CLR via C#-ban van egy érdekes rész (844-846 oldal a harmadik kiadásban). Ebből kiderül, hogy amit írtam megoldásnak, az nem feltétlenül jó. Ugye ezt írtam:

    var temp = new Dictionary
    FillTradingHours(temp, “HOL”);
    holidayDays = temp;

    Az a gond a fenti pár sorral, hogy elviekben a jitter átrendezheti az írások-olvasások sorrendjét. Ha elvonatkoztatunk a többszálúságtól, akkor a következő végrehajtási sorrend ugyanazt az eredményt adja:

    var temp = new Dictionary
    holidayDays = temp;
    FillTradingHours(temp, “HOL”);

    A fordító nem fog törődni a többszálúsággal, ha csak mi nem jelöljük külön. Ha pont úgy jön ki a lépés, hogy észreveszi a jitter, hogy ha úgyis benne van mondjuk az eax-ban a temp értéke, akkor gyorsan elmenti még a holidayDays-be is, mert a FillTradingHours tönkrevágja a regisztereket, és akkor újra kéne olvasni. Ekkor a másodjára leírt sorrendet fogja csinálni, és ugyanaz a szívás lesz, mint eredetileg, mivel még nincs feltöltve a dictionary, de a referenciája már elérhető a holidayDays-ben.

    Ezt a sorrendfelcserélést három módon lehet megakadályozni. Az egyik, amit te csináltál, hogy volatile-re raktad a holidayDays-t. Ekkor a holidayDays írása előtt leírt írások és olvasások a holidayDays írása előtt fognak befejeződni, tehát a fordító és a jitter nem fogja a FillTradingHours-t levinni a holidayDays írása alá, mivel az olvassa a temp-et, és korábban van a kódban, mint a holidayDays írása. A volatile kulcsszóval az az apró gond, hogy minden holidayDays elérésnél volatile write és volatile read lesz, ami kicsit lassítja a programot minden esetben, amikor később hivatkozod ezt a változót.

    Második megoldás, hogy kézzel kiírjuk a volatile write-ot (és nem használunk volatile kulcsszót)

    var temp = new Dictionary
    FillTradingHours(temp, “HOL”);
    Thread.VolatileWrite(ref holidayDays, temp);

    Ekkor a teljesítményveszteség csak ennél az írásnál lép fel, a későbbi olvasásoknál nem, de ezen a részen ugyanúgy nem variálja át a sorrendet a fordító, mint a volatile kulcsszónál.

    Harmadik, hogy ugyanezt csináljuk az Interlocked.Exchange()-dzsel. A CLR via C#-ban ez a példa van megadva, de arra nem jöttem rá, hogy ez miért jobb, mint a VolatileWrite(). Akkor lehet jobb, ha vannak még műveletek a következő sorokban, mert az Interlocked.Exchange() azt is megakadályozza, hogy ezek a műveletek egy esetleges optimalizálás során felülre kerüljenek (mivel full fence-t csinál), de ezt a double lock-os példában nem jön elő.

    Én egyébként ezekután bennehagynám a volatile kulcsszót, ahogy tetted, és nem törödnék a 0.03 ms overhead-del olvasásonként.

  • Tóth Viktor January 22, 2011

    Végigolvasva mit írtam, nem biztos, hogy összeáll a kép. Szóval három dolog van a többszálú programoknál, amire figyelni kell:

    1) Ha több magos processzor van, akkor a cache-ek miatt hiába ír az egyik mag egy memóriaterületet, a másik mag ezt nem fogja látni.

    2) A compilerek hajlamosak memóriaértékeket regiszterekben tartani, viszont ha egy másik szál a memóriát írta, akkor az első szál ezt nem veszi észre (mivel ő el van a regiszterbe töltött értékkel)

    3) A comiplerek hajlamosak átrendezni az írások és olvasások sorrendjét, ami egy szált tekintve nem jelent változást a végső működésben, de egy második szálat számbavéve az átrendezésnek csúnya mellékhatásai lehetnek.

    Az a zavaró a volatile meg lock meg társai témakörben, hogy mind a három problémát egyszerre kezelik, de kicsit másképpen, és az ember nem tudja mikor miért mit használjon. A lock például enternél érvényteleníti a cache-t, szóval a lockon belüli rész tényleg a valós memóriaállapotot fogja olvasni, ezzel megoldást adva az 1) problémára. Ugyanakkor nem engedi úgy optimalizálni a fordítót, hogy a lock elé kerüljenek olvasási, és mögé írási műveletek (3-ik probléma, illetve részben második, mert a regisztereket is emiatt újraolvassa, de a lockon belül már nem olvassa újra mindig). A volatile kulcsszóval jelölt változóknál sem fog a jitter elé és mögé vinni műveleteket (tehát a volatile kulcsszóval jelölt változó körül nem rendezget, 3-as probléma), plusz nem fog regiszterbe tárolni értékeket (2), de a lockkal ellentétben soha nem fog, ebben jóval szigorúbb.

    Az általam kritizált cikk, ami meg a lock-ot kritizálta, azt gondolta, hogy a lock hatása az lesz, hogy a lockon belül nem fog regiszterbe optimalizálni (ugyanúgy, mint a volatilenél). Mivel látta a cikkíró, hogy de, mégis regiszterbe fog tartani értékeket, azt gondolta, hogy a lock nem végzi el a feladatát, ezért kell volatile kulcsszó. Viszont a locknak más a célja, ő csak egy határt jelöl ki (fence) és számára az érdekes, hogy a határ előtt és mögött kiküszöbőlje 1-2-3-at.

    Ha jó helyen használjuk a lockot, akkor amúgy tényleg felesleges a lock-on belül kiiktatni a regiszterbe optimalizálást, hiszen elvileg azért van lock-unk, hogy a védett erőforráshoz más ne férjen hozzá, ha meg más nem fér hozzá, akkor nem fog az értéke megváltozni, szóval nyugodtan lehet regiszterben tartani.

    Emiatt tehát nem kell a volatile, ami miatt mégis kell, az egy másik (regiszteroptimalizációtól (2) független) probléma, az utasítássorrend megváltoztatása (3). A lock csak a belépés előtt és utáni határra figyel, hogy lockon belül mi történik, az neki mindegy. Viszont a Soci féle példában (illetve amit én megoldásnak gondoltam rá) olyan helyzet áll elő, hogy a compiler jószándékkal bár, de keresztbe tehet, mert a lock hatóköre az átrendezés tiltására oda már nem terjed ki. A volatile kulcsszó tehát mégis kell, de nem azért, amiért a linkkelt cikk, (ami lock-ot félreértelmezte) mondja, hanem egy másik probléma miatt.

    Hát, nem tudom, most világosabb-e…

  • Szindbad January 24, 2011

    Ez azt mondja, hogy az explicit memorybarrier hatekonyabb, mint a volatile:

    http://en.wikipedia.org/wiki/Double-checked_locking#Usage_in_Microsoft_.NET_.28Visual_Basic.2C_C.23.29

  • Tóth Viktor January 24, 2011

    Szindbad: igen, mint ahogy írtam is: ” A volatile kulcsszóval az az apró gond, hogy minden holidayDays elérésnél volatile write és volatile read lesz, ami kicsit lassítja a programot minden esetben, amikor később hivatkozod ezt a változót.”

    Viszont nehezebb elrontani, meg elfelejteni :)

    Egyébként megvan, hogy a CLR via C# könyv miért interlocked-et használ VolatileWrite helyett: az interlocked-nek van template-es verziója, emiatt minden típusnak tud értéket adni, a VolatileWrite meg csak object-tel működik. Tehát ez, amit írtam:

    Thread.VolatileWrite(ref holidayDays, temp);

    le sem fordul ez viszont:

    Interlocked.Exchange(ref holidayDays, temp);

    megy szépen.

  • Soczó Zsolt February 1, 2011

    Huh, most vettem észre mennyit írtál Viktor, köszi, ezt megrágom, aztán csinálok belőle egy új bejegyzést, linkekkel definit forrásokra.

  • a. February 8, 2011

    a masik problema a peldaval, hogy static valtozon lockol, de a property nem static, igy az adott objektum kulonbozo peldanyainak elerese is szerializalva lesz amig inizializalnak.