How to Thread-Safe?

also ich bin gerade am "Umprogrammieren"...und da fällt mir auf, dass wir bisher garnichts für die Sicherheit bei Multi-Threading getan haben (also Synchronisation,...).
Die Struktur sieht so aus:
Webseite ruft per WCF die BLL in einem Windows Service auf. Diese ruft ihrerseits die DAL auch in dem selben Windows Service auf, die DAL ist so gestaltet, dass sie die Daten ständig im Cache behält und ggf für Backups in XML-Dateien dumpt.
zum Code (hoffe es ist nicht schlimm, dass ich zu einem anderen Forum verlinke, aber ich wollte den Code nicht extra rauskopieren...würde den Thread hier nur unnötig aufblähen)
Ein möglicher problematischer Pseudo-Code wäre zb (in der BLL):

Rohstoffe guthaben= DAL.GibRohstoffeVonUser(userid);
if(guthaben.HatgenügendRohstoffe(Kosten) == true)
{
Ziehe Rohstoffe ab, baue Gebäude,...
}
Da nach der Überprüfung der Anzahl der Rohstoffe der Thread "gepaust" werden könnte, und dann die Methode nochmal aufgerufen wird. Angenommen der 2. Aufruf kommt auch min. bis nach der Überprüfung, dann werden 2 mal Rohstoffe abgezogen, obwohl es sein kann, dass nach dem ersten Abziehen nicht mehr genug Rohstoffe da sind um das 2. mal sie abzuziehen (da ja keine Überprüfung mehr stattfindet).
Wie kann man so etwas verhindern?
1. Ein lock über "guthaben" würde ja nichts bringen, da ein 2. Methodenaufruf ein 2. "guthaben" erzeugen würde.
2. Ganz Mutli-Threading verhindern will ich auch nicht (oder hab ich nur zuviel Angst um die Performance, wenn der WCF-Service die Anfragen nur nacheinander abarbeitet?).
3. Würde ja nur noch bleiben, dass ich irgendwo speichere welche Guthaben von welchen Usern gerade bearbeitet werden und wenn noch eine Anfrage an ein gerade bearbeitetes Guthaben geht, dass diese aufgeschoben wird. Das müsste dann aber für fast alle Dinge gemacht werden (--> Keine Lust )
EDIT: ist mir gerade noch eingefallen... gefällt mir eigentlich schon ganz gut
4.

lock(this)
{
Rohstoffe guthaben= DAL.GibRohstoffeVonUser(userid);
bool genügend = guthaben.HatgenügendRohstoffe(Kosten);
}
if(genügend == true)
{
Ziehe Rohstoffe ab, baue Gebäude,...
}
Und das alles in einem Singleton... (sodass während des lock alles andere aufgeschoben wird)
Gibts noch was?

4. gibts doch bestimmt in jeder komplexen Hochsprache. In Java z.B. kann man Methoden oder Blöcke synchronisieren und so gezielt eine Ausführung nacheinander ablaufen lassen - ohne großen Aufwand.

> lock(this)
Nicht sehr empfehlenswert.
msdn2.microsoft.com/de-de/magazine/cc188793(en-us).aspx
Lieber nach Methode 10 verfahren und 8 Bytes Overhead besitzen.

ok thx
also Undeadable du meinst mit Methode 10, dass verwenden von privaten object-locks?

private readonly object locker = new Object();
lock(locker)
...

Exakt.
Ansonsten kann es sein, dass du Dead-Locks bekommst, wenn irgendeine Funktion/Methode/Klasse deine Instanz selbst lockt, obwohl du es nicht erwartest.

gut jetzt hab ich das mal weitergedacht, und ich möchte ja nicht den ganzen Daten-Layer blocken, nur weil die Userdaten (Passwort, Email,...) eines Users gerade geupdatet werden (als Beispiel)... da kann ruhig zb. ein Gebäude gebaut werden oder die Produktion/Guthaben verändert.
Also würde ich jeden Bereich einen eigenen Objekt-Locker (wie oben im Beispiel) zuteilen (einen für die Gebäude, einen für die Userdaten,...) und dann immer wenn ich etwas mit dem entsprechenden Gebiet mache, das locken.
Da hab ich aber auch meine Probleme mit:

BauGebäude()
{
//Muss gelockt werden, da zwischen Überprüfung und Abbuchen kein andere Thread an das Guthaben darf
lock(Produktion)
{
Überprüfe ob genügend Rohstoffe
lock(Gebäude)
{
Überpüfe ob genügend Platz auf dem Planeten

andere Überprüfungen //leider fast alle mit ihrem eigenen Lock

Wenn alle Überprüfungen positiv, Rohstoffe abziehen und Platz auf dem Planeten verringern
}
}
}
Sodass bei solchen Operationen trotzdem fast alle Bereiche gelockt sind, obwohl man sie eigentlich nicht braucht (nur noch andere Bereiche überprüfen muss)...
und was passiert bei solchem Code (auch ohne die Unterteilung in Bereiche möglich)

BaueGebäude
{
//Lock nötig um keinen Thread zwischen Überprüfung und Abziehen dazwischenfunken zu lassen
lock(lockerProduktion)
{
ÜberprüfeGuthaben()
//Guthaben abziehen... wird aber sowieso nie aufgerufen
}
}
ÜberprüfeGuthaben()
{
//In dem Beispiel vllt nicht nötig...gibt aber sicherlich Fälle, wo dies nötig ist
lock(lockerProduktion)
...
}
müsste ja zu einem Dead-Lock führen oder?
Ich hasse es

> und was passiert bei solchem Code (auch ohne die Unterteilung in Bereiche möglich)
Ein Thread selbst kann sich in .Net nicht deadlocken.
> Da hab ich aber auch meine Probleme mit:
Dann kannst du eventuell überlegen, ob du nicht einzelnen Objektinstanzen ein Lock zuweist. Dort haust du dir aber schneller einen Deadlock rein, als dir lieber ist.
Eine andere Alternative wäre das Nutzen des ReadWriteLocks. Ab .Net 3.5 gibt es einen SlimReadWriteLock (oder so ähnlich, musst du mal suchen, hier in der Firma muss ich mit Tux-Kram arbeiten), der einen sehr performanten ReadWrite-Lock implementiert.
Ich persönlich habe mein Spiel (jungfrauenspiel) mit einem globalen Lock gesperrt. Nicht optimal, aber war für mich gangbar, da ich maximal 200 Spieler erwartet hatte.
Bei meinem neuen Projekt sperre ich die einzelnen Objekte mit einem ReadWriteLock.
METACODE:
using ( oTown.GetWriteLock() )
{
bool bHasResearch;
using ( oPlayer.GetReadLock() )
{
bHasResearch = oPlayer.HasResearchForTown (...);
}
if ( bHasResearch ) { oTown.ConstructBuilding(); }
}
Die Klasse ReadWriteLock habe ich wegabstrahiert, so dass ich das elegante IDisposable-Muster mit using nutzen kann.

Ein Thread selbst kann sich in .Net nicht deadlocken.

Gut, das hab ich gehofft aber nicht wirklich gewusst
Danke mit dem Tipp (ReaderWriterLockSlim)!
Besonders geil finde ich den Modus UpgradeableRead...damit ist genau der oben angesprochene Fall abdeckt (evt wird ja garnicht geschrieben, sondern nur gelesen).
Ist es jetzt noch ratsam, das alles auf verschiedene Bereiche auszubauen, oder reicht es mit den Funktionen global zu arbeiten?
Vielen, vielen Dank für den Tipp (den hätte ich schon 2 Tage vorher gebraucht^^)

Original von altertoby

BaueGebäude
{
//Lock nötig um keinen Thread zwischen Überprüfung und Abziehen dazwischenfunken zu lassen
lock(lockerProduktion)
{
ÜberprüfeGuthaben()
//Guthaben abziehen... wird aber sowieso nie aufgerufen
}
}
ÜberprüfeGuthaben()
{
//In dem Beispiel vllt nicht nötig...gibt aber sicherlich Fälle, wo dies nötig ist
lock(lockerProduktion)
...
}
müsste ja zu einem Dead-Lock führen oder?

Wenn, dann zu einem Self-Lock aber auch nur dann, wenn der Lock nicht rekursiv ist.
Ein dead lock kann dann auftreten, wenn Du mehr als einen Lock/Mutex verwendest und diese Locks in zwei unterschiedlichen Code-Pfaden in unterschiedlichen Reihenfolgen gelockt werden. Dann kann der Fall auftreten, dass Thread 1 Mutex A hat und auf Mutex B wartet und Thread 2 Mutex B hat und auf Mutex A hat.
Die Verwendung von rekursiven Locks verhindert das nicht und wiegt den Anwender bloß in falscher Sicherheit.

@Todi42: Ok dann halt Self-Lock Aber dank TheUndeadable, ist das ja geklärt, dass dies in .Net nicht möglich ist!
@All: Ich hab jetzt mal die Idee mit dem using von Undeadable umgesetzt... ich poste das eigentlich nur mal zur Verfollständigung und dass jemand der mehr Ahnung von der Materie hat, das mal auf Fehler untersuchen kann
Meine Tests haben geklappt

//Erweitert den ReaderWriterLockSlim um die "GetXXX"-Methoden
public static class LockExtender
{
public static LockHelper GetWriteLock(this ReaderWriterLockSlim slim)
{
slim.EnterWriteLock();
LockHelper helper = new LockHelper();
helper.Locker = slim;
helper.Modus = Mode.Write;
return helper;
}
public static LockHelper GetReadLock(this ReaderWriterLockSlim slim)
{
slim.EnterReadLock();
LockHelper helper = new LockHelper();
helper.Locker = slim;
helper.Modus = Mode.Read;
return helper;
}
public static LockHelper GetUpgradeableReadLock(this ReaderWriterLockSlim slim)
{
slim.EnterUpgradeableReadLock();
LockHelper helper = new LockHelper();
helper.Locker = slim;
helper.Modus = Mode.UpgradeableRead;
return helper;
}
}
//Klasse um das Using zu ermöglichen (und das Wechseln des Modus)
public class LockHelper : IDisposable
{
public ReaderWriterLockSlim Locker { get; set; }
public Mode Modus { get; set; }
public void Dispose()
{
switch (Modus)
{
case Mode.Read:
Locker.ExitReadLock();
break;
case Mode.UpgradeableRead:
Locker.ExitUpgradeableReadLock();
break;
case Mode.Write:
Locker.ExitWriteLock();
break;
case Mode.WriteAfterUpgradeable:
Locker.ExitWriteLock();
Locker.ExitUpgradeableReadLock();
break;
case Mode.ReadAfterUpgradeable:
Locker.ExitReadLock();
break;
default:
throw new NotImplementedException("Modus " + Modus.ToString() + "not implemented");
}
}
public void UpgradeToWrite()
{
if (Modus != Mode.UpgradeableRead)
throw new NotSupportedException("To upgrade the Lock to write, the locker have to be in the UpgradeableRead Modus");
Modus = Mode.WriteAfterUpgradeable;
Locker.EnterWriteLock();
}
public void DowngradeToRead()
{
if (Modus != Mode.UpgradeableRead)
throw new NotSupportedException("To upgrade the Lock to write, the locker have to be in the UpgradeableRead Modus");
Modus = Mode.ReadAfterUpgradeable;
Locker.EnterReadLock();
Locker.ExitUpgradeableReadLock();
}
}
//Die verschiedenen Modi des Lockers
public enum Mode
{
Read,
UpgradeableRead,
Write,
WriteAfterUpgradeable,
ReadAfterUpgradeable
}
Und hier ist noch der kleine Test (Konsolenanwendung)

static void Main(string[] args)
{
Console.WriteLine("Test started...");
Thread thread1 = new Thread(Method);
thread1.Name = "1";
thread1.Start();
Thread thread2 = new Thread(Method);
thread2.Name = "2";
thread2.Start();
}
public static string text = "Hallo ";
public static ReaderWriterLockSlim locker = new ReaderWriterLockSlim();
public static void Method()
{
Console.WriteLine("Method was called by " + Thread.CurrentThread.Name);
using (LockHelper helper = locker.GetUpgradeableReadLock())
{
if (text == "Hallo ")
{
helper.UpgradeToWrite();
text += "Tobias";
}
else
{
helper.DowngradeToRead();
}
Console.WriteLine(text);
Thread.Sleep(5000);
}
Console.WriteLine("Method Call by " + Thread.CurrentThread.Name + " finished");
return;
}