Threadsicherheit in Get und Set bei statischen Eigenschaften

Hallo,
ich suche nach einer Möglichkeit meine Eigenschaft im Get und Set Zweig threadsicher zu machen. Bei Eigenschaften die ich nur erhöhen möchte klappt es mit Interlocked.Increment im Set Zweig. Bei allen anderen Eigenschaften habe ich diese Lösung beim googlen gefunden und getestet aber sie Funktioniert nicht. Wisst ihr wieso?

    public partial class MyTestClass
    {
        private static object _MyObject = new object();
        private static int _MyProperty = 0;


        public static int MyProperty
        {
            get
            {
                lock (_MyObject)
                {
                    return _MyProperty;
                }
            }
            set
            {
                lock (_MyObject)
                {
                    _MyProperty = value;
                }                
            }

        }
   }

Vorweg: Statische Klassen mit einem statischen Zustand, der sich auch noch aus verschiedenen Threads heraus ändern kann - das ist garantiert eine ganz furchtbar schlechte Idee 😉

Und das Abrufen/Zuweisen einer Variable ist nicht das Problem, sondern was Du danach damit machst.
Beispiel:

if (_myIndex >= 0)
{
    var item = _myArray[_myIndex];
}

Hier kann sich zwischen dem if und der Verwendung im Array der Wert vom Index ändern.
Es kann also sein, dass der Array-Zugriff trotz der Prüfung zu einer IndexOutOfRangeException führt.

Das würde gehen:

var myIndex = _myIndex;

if (myIndex >= 0)
{
    var item = _myArray[myIndex];
}

Das Problem umgehst Du mit der Property aber sowieso, da sich der Wert, da Du beim Abrufen der Property ja das gleiche tust.
Bei komplexeren Abläufen (z.B. wenn mehrere Werte zusammen verarbeitet werden müssen) verwendet man dann lock, womit Du erzwingen kannst, dass nur ein Thread auf einmal ein Stück Code ausführt. Und dann gibt's noch diverse Kontroll-Strukturen (z.B. AutoResetEvent).

Aber das alles brauchst Du hier gar nicht, da deine Property nichts tut, was kaputt gehen könnte - sie ist bereits threadsafe.
Dafür müssen alle Klassen, die diese Property nutzen, sich um verschiedene Threads kümmern.

Wenn Du etwas brauchst ist das in den aller aller aller meisten Fällen ein klassischer Software Design Fehler, der sich mit anderen Pattern vermutlich einfach lösen lässt.
Dazu musst Du aber beschreiben, was Du eigentlich tun willst.
Das Ziel ist ja die Ursache zu behandeln, und nicht das Symptom.

Die Handhabung des Locks ist hier aber prinzipiell korrekt, wenn das auch beim Return (nicht unbedingt) Sinn macht.

Danke für eure Antworten.

Wenn ich das richtig verstehe sagt ihr, dass meine Eigenschaft threadsicher ist. Problem ist ich habe es mit einer parallel.for schleife getestet. Dort habe ich 100mal die Eigenschaft um eins erhöht und am Ende kam beim Ergebnis nicht 100 raus. Sondern unterschiedliche Werte wie 97,78,67... Deshalb frag ich mich wieso nicht 100 raus kommt. Ihr sagt ja mein lock ist richtig.

Weiterhin bin ich mir unsicher was ihr mit Design-Fehler meint. Darf/Sollte man keine Statischen Eigenschaften benutzen?

Darf/Sollte man keine Statischen Eigenschaften benutzen?

Darf man - wenn es sinn macht; viele Fälle sind das nicht.
Oft verwendet man das aus Faulheit; und hat am Ende nicht testbaren, schwer wartbaren Code, der auch noch hohe Abhängigkeiten hat.

Daher kann man pauschal sagen: static für Logik ist schlecht.

Problem ist ich habe es mit einer parallel.for schleife getestet.

Vermutlich, weil Du nicht zu Ende gedacht hast und in einen klassische Race Condition gelaufen bist.
Ich nehme mal an, dass Dein Code folgendermaßen aussieht:

t.MyProperty = t.MyProperty + 1;

Hier werden also zwei Locks angewandt: Lesen und Schreiben. Während Du aber die Addition durchführst erfolgt eine andere und Du läufst ist eine Race Condition.
Das ist nur vermeidbar, wenn das ganze Objekt gelockt wird - so ist das halt eine wirklich ganz klassische Race Condition.

Möglich ist auch die entspreche Kalkulation zu locken, um eine Race Condition zu vermeiden

    private object _addLock = new object();
    public void Add(int i)
    {
        lock (_addLock)
        {
            MyProperty += i;
        }
    }

Hingegen machst Du:

    public void Add(int i)
    {
        int aktuellerWert;

        lock (_lock)
        {
            aktuellerWert = MyProperty;
        }

        // Hier bist Du unsynchronisiert, Ursache Deines Problems
       aktuellerWert = aktuellerWert  + 1;

        lock (_lock)
        {
            MyProperty = aktuellerWert ;
        }
    }

Du hast schließlich nur die Zuweisungen Thread-Safe umgesetzt, aber nicht die Kalkulation.

Aber wie gesagt; ich vermute, dass Dein Code an anderer Stelle schon nicht stimmt und einen Designfehler hat.

Edit: sorry für die beiden Edits Palladin 🙂

Problem ist ich habe es mit einer parallel.for schleife getestet. Dort habe ich 100mal die Eigenschaft um eins erhöht und am Ende kam beim Ergebnis nicht 100 raus. Sondern unterschiedliche Werte wie 97,78,67... Deshalb frag ich mich wieso nicht 100 raus kommt. Ihr sagt ja mein lock ist richtig.

Du kannst die Property so threadsafe machen wie Du willst, das beeinflusst aber nicht die Reihenfolge, in der die Werte für die Property gesetzt werden.
Parallel.For nimmt die 100 und teilt sie auf deine Anzahl CPU-Kerne auf, sodass jeder Kern eine gleichmäßige Anzahl Aufgaben abzuarbeiten hat.
Das heißt aber auch, dass keine Reihenfolge sichergestellt ist und als letzter Wert alles bei raus kommen kann.

PS:

Ich bin von ungefähr diesem Code ausgegangen:

Parallel.For(0, 100, i => MyTestClass.MyProperty = i);

Aber Abts Annahme klingt realistischer, entsprechend passt auch seine Erklärung besser.

Was ich interessant finde, dass einige Antworten eingehen obwohl bis zur Erklärung nun wirklich niemand wissen konnte was da "nicht funktioniert" 😉

Das ist generell das Problem, wenn nicht genug Informationen zur Verfügung gestellt werden.
Aber deswegen hab wir ja eigentlich [Hinweis] Wie poste ich richtig? - aber trotzdem versucht man ja zu helfen....

Was schief is, das wusste man ja schon ungefähr.

Ich dachte meine Informationen im ersten Beitrag reichen da ich davon ausging das ich was im Get und Set zwei falsch gemacht habe. Tatsächlich wie ihr sagt, lag der Fehler in der Operation. Danke auf jeden Fall für die Hilfe.