Multi-Threading-Frage - Hinzufügen eines Elements zu einer statischen Liste

Question

Okay, Neuling Multi-Threading Frage:

Ich habe eine Singleton-Klasse. Die Klasse hat eine statische Liste und funktioniert im Wesentlichen wie folgt:

class MyClass {
    private static MyClass _instance;
    private static List<string> _list;
    private static bool IsRecording;

    public static void StartRecording() {
        _list = new List<string>();
        IsRecording = true;
    }

    public static IEnumerable<string> StopRecording() {
        IsRecording = false;
        return new List<string>(_list).AsReadOnly();
    }

    public MyClass GetInstance(){

    }

    public void DoSomething(){
        if(IsRecording) _list.Add("Something");
    }
}

Grundsätzlich kann ein Benutzer StartRecording() aufrufen, um eine Liste zu initialisieren, und dann können alle Aufrufe einer Instanz-Methode der Liste etwas hinzufügen. Allerdings können mehrere Threads eine Instanz von MyClass halten, so dass mehrere Threads Einträge zur Liste hinzufügen können.

Allerdings sind sowohl das Erstellen als auch das Lesen von Listen Einzeloperationen, so dass das übliche Reader-Writer-Problem in Situationen mit mehreren Threads nicht zutrifft. Das einzige Problem, das ich sehen könnte, ist, dass die Einfügereihenfolge seltsam ist, aber das ist kein Problem.

Kann ich den Code so belassen, wie er ist, oder muss ich irgendwelche Vorkehrungen für Multithreading treffen? Ich sollte hinzufügen, dass es sich in der realen Anwendung nicht um eine Liste von Strings, sondern um eine Liste von benutzerdefinierten Objekten handelt (der Code lautet also _list.Add(new Object(somedata))), aber diese Objekte enthalten nur Daten, keinen Code außer einem Aufruf von DateTime.Now.

Bearbeiten: Klarstellungen nach einigen Antworten: DoSomething kann nicht statisch sein (die Klasse ist hier abgekürzt, es gibt eine Menge Dinge, die Instanzvariablen verwenden, aber diese werden vom Konstruktor erzeugt und dann nur gelesen). Ist es gut genug zu tun

lock(_list){
    _list.Add(something);
}

and

lock(_list){
    return new List<string>(_list).AsReadOnly();
 }

oder brauche ich einen tieferen Zauber?

Answer 1

Sie müssen die _Liste unbedingt sperren. Und da Sie mehrere Instanzen für _list erstellen, können Sie nicht auf _list selbst sperren, aber Sie sollten etwas wie verwenden:

private static object _listLock = new object();

Nebenbei bemerkt, möchte ich ein paar bewährte Praktiken beachten:

• DoSomething() kann, wie gezeigt, statisch sein und sollte es auch sein.

• für Bibliotheksklassen wird empfohlen, statische Mitglieder thread-sicher zu machen, das gilt für StartRecording() , StopRecording() y DoSomething() .

Ich würde auch StopRecording() einstellen. _list = null und prüfen Sie es auf Null in DoSomething() .

Und bevor Sie fragen: Das alles nimmt so wenig Zeit in Anspruch, dass es wirklich keine Leistungsgründe gibt. pas um es zu tun.

Answer 2

Sie müssen die Liste sperren, wenn mehrere Threads zu ihr hinzugefügt werden.

Ein paar Beobachtungen...

Vielleicht gibt es einen Grund, es nicht zu tun, aber ich würde vorschlagen, die Klasse statisch zu machen und damit alle ihre Mitglieder statisch. Es gibt keinen wirklichen Grund, zumindest nach dem, was Sie gezeigt haben, von den Clients von MyClass zu verlangen, die Methode GetInstance() aufzurufen, nur damit sie eine Instanzmethode, in diesem Fall DoSomething(), aufrufen können.

Ich sehe nicht, was jemanden daran hindert, die Methode StartRecording() mehrfach aufzurufen. Sie könnten in Erwägung ziehen, eine Prüfung einzufügen, so dass Sie, wenn die Aufzeichnung bereits läuft, keine neue Liste erstellen und damit allen den Teppich unter den Füßen wegziehen.

Wenn Sie schließlich die Liste sperren, sollten Sie dies nicht auf diese Weise tun:

static object _sync = new object();
lock(_sync){
    _list.Add(new object(somedata));
}

Minimieren Sie die Zeit, die innerhalb der Sperre verbracht wird, indem Sie die Erstellung neuer Objekte außerhalb der Sperre verschieben.

static object _sync = new object();
object data = new object(somedata);
lock(_sync){
    _list.Add(data);
}

EDIT

Sie sagten, dass DoSomething() nicht statisch sein kann, aber ich wette, es kann. Du kannst immer noch ein Objekt von MyClass innerhalb von DoSomething() für alle instanzbezogenen Dinge verwenden, die du tun musst. Aber vom Standpunkt der Programmierbarkeit aus gesehen, sollten Sie nicht verlangen, dass die Benutzer von MyClass zuerst GetInstance() aufrufen. Bedenken Sie dies:

class MyClass {
    private static MyClass _instance;
    private static List<string> _list;
    private static bool IsRecording;
    public static void StartRecording()
    {
        _list = new List<string>();
        IsRecording = true;
    }
    public static IEnumerable<string> StopRecording()
    {
        IsRecording = false;
        return new List<string>(_list).AsReadOnly();
    }
    private static MyClass GetInstance() // make this private, not public
    {   return _instance;    }
    public static void DoSomething()
    {
        // use inst internally to the function to get access to instance variables
        MyClass inst = GetInstance();
    }
}

Auf diese Weise können die Benutzer von MyClass von

MyClass.GetInstance().DoSomething();

zu

MyClass.DoSomething();

Answer 3

.NET-Sammlungen sind nicht vollständig thread-sicher. Von MSDN : "Mehrere Leser können die Sammlung zuverlässig lesen; jede Änderung der Sammlung führt jedoch zu undefinierten Ergebnissen für alle Threads, die auf die Sammlung zugreifen, einschließlich der Leser-Threads." Sie können die Vorschläge auf dieser MSDN-Seite befolgen, um Ihre Zugriffe thread-sicher zu machen.

Ein Problem, auf das Sie mit Ihrem aktuellen Code wahrscheinlich stoßen würden, ist, wenn StopRecording aufgerufen wird, während sich ein anderer Thread in DoSomething befindet. Da die Erstellung einer neuen Liste aus einer bestehenden Liste eine Aufzählung erfordert, werden Sie wahrscheinlich auf das alte Problem "Sammlung wurde geändert; Aufzählungsvorgang kann nicht ausgeführt werden" stoßen.

Fazit: Üben Sie sicheres Einfädeln!

Answer 4

Es ist möglich, wenn auch knifflig, eine verknüpfte Liste zu schreiben, die gleichzeitige Einfügungen von mehreren Threads ohne Sperre erlaubt, aber das ist es nicht. Es ist einfach nicht sicher, _list.Add parallel aufzurufen und auf das Beste zu hoffen. Je nachdem, wie es geschrieben ist, könnten Sie einen oder beide Werte verlieren oder die gesamte Struktur beschädigen. Sperren Sie sie einfach.