List.Add () Thread-Sicherheit

84

Ich verstehe, dass eine Liste im Allgemeinen nicht threadsicher ist. Ist es jedoch falsch, einfach Elemente zu einer Liste hinzuzufügen, wenn die Threads niemals andere Vorgänge in der Liste ausführen (z. B. das Durchlaufen)?

Beispiel:

List<object> list = new List<object>();
Parallel.ForEach(transactions, tran =>
{
    list.Add(new object());
});
e36M3
quelle
3
Exaktes Duplikat der Thread-Sicherheit
JK.
Ich habe einmal eine Liste <T> verwendet, um neue Objekte aus mehreren parallel ausgeführten Aufgaben hinzuzufügen. Manchmal, sehr selten, endete das Durchlaufen der Liste nach Abschluss aller Aufgaben mit einem Datensatz, der null war. Wenn keine zusätzlichen Threads beteiligt gewesen wären, wäre dies praktisch unmöglich gewesen. Ich denke, als die Liste ihre Elemente intern neu zuordnete, um sie zu erweitern, hat ein anderer Thread sie irgendwie durcheinander gebracht, indem er versucht hat, ein anderes Objekt hinzuzufügen. Es ist also keine gute Idee, dies zu tun!
Osmiumbin

Antworten:

72

Hinter den Kulissen passieren viele Dinge, einschließlich der Neuzuweisung von Puffern und des Kopierens von Elementen. Dieser Code wird eine Gefahr verursachen. Ganz einfach, es gibt keine atomaren Operationen beim Hinzufügen zu einer Liste, zumindest muss die Eigenschaft "Länge" aktualisiert werden, und das Element muss an der richtigen Stelle eingefügt werden, und (wenn es eine separate Variable gibt) der Index muss aktualisiert werden. Mehrere Threads können übereinander treten. Und wenn ein Wachstum erforderlich ist, ist noch viel mehr los. Wenn etwas in eine Liste schreibt, sollte nichts anderes darin lesen oder schreiben.

In .NET 4.0 gibt es gleichzeitige Sammlungen, die handlich threadsicher sind und keine Sperren erfordern.

Talljoe
quelle
Das macht durchaus Sinn, dafür werde ich mir auf jeden Fall die neuen Concurrent-Kollektionen ansehen. Danke dir.
e36M3
9
Beachten Sie, dass es keinen integrierten ConcurrentListTyp gibt. Es gibt gleichzeitig Taschen, Wörterbücher, Stapel, Warteschlangen usw., aber keine Listen.
LukeH
10

Ihr aktueller Ansatz ist nicht threadsicher - ich würde vorschlagen, dies insgesamt zu vermeiden -, da Sie im Grunde genommen eine Datentransformation durchführen. PLINQ könnte ein besserer Ansatz sein (ich weiß, dass dies ein vereinfachtes Beispiel ist, aber am Ende projizieren Sie jede Transaktion in einen anderen "Zustand" " Objekt).

List<object> list = transactions.AsParallel()
                                .Select( tran => new object())
                                .ToList();
Glassplitter
quelle
Ich habe ein übermäßig vereinfachtes Beispiel vorgestellt, um den Aspekt von List.Add hervorzuheben, an dem ich interessiert war. Mein Parallel.Foreach wird in der Tat eine Menge Arbeit leisten und keine einfache Datentransformation sein. Vielen Dank.
e36M3
4
Gleichzeitige Sammlungen können Ihre parallele Leistung beeinträchtigen, wenn sie nicht benötigt werden. Sie können auch ein Array mit fester Größe verwenden und die Parallel.ForeachÜberladung verwenden, die den Index berücksichtigt. In diesem Fall manipuliert jeder Thread einen anderen Array-Eintrag, und Sie sollten sicher sein.
BrokenGlass
6

Es ist nicht unvernünftig zu fragen. Es gibt Fälle, in denen Methoden, die in Kombination mit anderen Methoden Probleme mit der Thread-Sicherheit verursachen können, sicher sind, wenn sie die einzige aufgerufene Methode sind.

Dies ist jedoch eindeutig kein Fall, wenn Sie den im Reflektor angezeigten Code betrachten:

public void Add(T item)
{
    if (this._size == this._items.Length)
    {
        this.EnsureCapacity(this._size + 1);
    }
    this._items[this._size++] = item;
    this._version++;
}

Selbst wenn EnsureCapacityes an sich threadsicher war (und dies ist es mit Sicherheit nicht), wird der obige Code eindeutig nicht threadsicher sein, wenn man bedenkt, dass gleichzeitige Aufrufe des Inkrementoperators zu Fehlschreibvorgängen führen können.

Entweder sperren, ConcurrentList verwenden oder eine Warteschlange ohne Sperre als Ort verwenden, an den mehrere Threads schreiben, und das Lesen daraus - entweder direkt oder durch Füllen einer Liste -, nachdem sie ihre Arbeit erledigt haben (ich gehe davon aus) Mehrere gleichzeitige Schreibvorgänge, gefolgt von Lesen mit einem Thread, sind hier Ihr Muster, gemessen an Ihrer Frage, da ich sonst nicht sehen kann, wie die Bedingung, bei der Adddie einzige aufgerufene Methode aufgerufen wird, von Nutzen sein könnte.

Jon Hanna
quelle
6

Wenn Sie List.addaus mehreren Threads verwenden möchten und sich nicht um die Reihenfolge kümmern möchten, benötigen Sie wahrscheinlich Listohnehin nicht die Indizierungsfunktion von a und sollten stattdessen einige der verfügbaren gleichzeitigen Sammlungen verwenden.

Wenn Sie diesen Rat ignorieren und nur tun add, können Sie den addThread sicher, aber in unvorhersehbarer Reihenfolge wie folgt machen:

private Object someListLock = new Object(); // only once

...

lock (someListLock)
{
    someList.Add(item);
}

Wenn Sie diese unvorhersehbare Bestellung akzeptieren, benötigen Sie wahrscheinlich, wie bereits erwähnt, keine Sammlung, die wie in indiziert indiziert werden kann someList[i].

tomsv
quelle
5

Dies würde Probleme verursachen, da die Liste über ein Array erstellt wird und nicht threadsicher ist. Je nachdem, wo sich die Threads befinden, kann es vorkommen, dass der Index außerhalb der Grenzen liegt oder einige Werte andere Werte überschreiben. Mach es im Grunde nicht.

Es gibt mehrere mögliche Probleme ... Nur nicht. Wenn Sie eine thread-sichere Sammlung benötigen, verwenden Sie entweder eine Sperre oder eine der System.Collections.Concurrent-Sammlungen.

Linkgoron
quelle
4

Ich habe mein Problem gelöst, ConcurrentBag<T>anstatt List<T>wie folgt:

ConcurrentBag<object> list = new ConcurrentBag<object>();
Parallel.ForEach(transactions, tran =>
{
    list.Add(new object());
});
alansiqueira27
quelle
2

Ist etwas falsch daran, einfach Elemente zu einer Liste hinzuzufügen, wenn die Threads niemals andere Vorgänge für die Liste ausführen?

Kurze Antwort: ja.

Lange Antwort: Führen Sie das folgende Programm aus.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading;

class Program
{
    readonly List<int> l = new List<int>();
    const int amount = 1000;
    int toFinish = amount;
    readonly AutoResetEvent are = new AutoResetEvent(false);

    static void Main()
    {
        new Program().Run();
    }

    void Run()
    {
        for (int i = 0; i < amount; i++)
            new Thread(AddTol).Start(i);

        are.WaitOne();

        if (l.Count != amount ||
            l.Distinct().Count() != amount ||
            l.Min() < 0 ||
            l.Max() >= amount)
            throw new Exception("omg corrupted data");

        Console.WriteLine("All good");
        Console.ReadKey();
    }

    void AddTol(object o)
    {
        // uncomment to fix
        // lock (l) 
        l.Add((int)o);

        int i = Interlocked.Decrement(ref toFinish);

        if (i == 0)
            are.Set();
    }
}
Bas Smit
quelle
@royi führen Sie dies auf einem Single-Core-Computer aus?
Bas Smit
Hallo, ich denke, es gibt ein Problem mit diesem Beispiel, da es das AutoResetEvent setzt, wenn es die Nummer 1000 findet. Da es diese Threads irgendwie verarbeiten kann, wann immer es will, kann es 1000 werden, bevor es beispielsweise 999 erreicht. Wenn Sie in der AddTol-Methode eine Console.WriteLine hinzufügen, sehen Sie, dass die Nummerierung nicht in Ordnung ist.
Dave Walker
@ Dave, es setzt das Ereignis, wenn ich == 0
Bas Smit
2

Wie bereits erwähnt, können Sie gleichzeitige Sammlungen aus dem System.Collections.ConcurrentNamespace verwenden. Wenn Sie eine davon verwenden können, wird dies bevorzugt.

Wenn Sie jedoch wirklich eine Liste wünschen, die nur synchronisiert ist, können Sie sich die SynchronizedCollection<T>Klasse in ansehen System.Collections.Generic.

Beachten Sie, dass Sie die System.ServiceModel-Assembly einschließen mussten, was auch der Grund ist, warum es mir nicht so gut gefällt. Aber manchmal benutze ich es.

maf-soft
quelle