Sécurité des threads List.Add ()

87

Je comprends qu'en général une liste n'est pas sûre pour les threads, mais y a-t-il quelque chose de mal à simplement ajouter des éléments dans une liste si les threads n'effectuent jamais d'autres opérations sur la liste (comme la parcourir)?

Exemple:

List<object> list = new List<object>();
Parallel.ForEach(transactions, tran =>
{
    list.Add(new object());
});
e36M3
la source
3
Copie exacte de la sécurité des threads List <T>
JK.
Une fois, j'ai utilisé un List <T> uniquement pour ajouter de nouveaux objets à partir de plusieurs tâches exécutées en parallèle. Parfois, très rare, lors de l'itération dans la liste une fois toutes les tâches terminées, il se retrouvait avec un enregistrement qui était nul, ce qui, si aucun thread supplémentaire n'avait été impliqué, il aurait été pratiquement impossible que cela se produise. Je suppose que, lorsque la liste réallouait en interne ses éléments pour se développer, un autre thread l'a gâchée en essayant d'ajouter un autre objet. Ce n'est donc pas une bonne idée de le faire!
osmiumbin
Exactement ce que je vois actuellement @osmiumbin en ce qui concerne un objet inexplicablement nul lors d'un simple ajout à partir de plusieurs threads. Merçi pour la confirmation.
Blackey

Réponses:

75

Dans les coulisses, beaucoup de choses se produisent, y compris la réaffectation des tampons et la copie d'éléments. Ce code entraînera un danger. Très simplement, il n'y a pas d'opérations atomiques lors de l'ajout à une liste, au moins la propriété "Length" doit être mise à jour, et l'élément doit être placé au bon endroit, et (s'il y a une variable séparée) l'index a besoin à mettre à jour. Plusieurs threads peuvent se piétiner. Et si une culture est nécessaire, il y en a beaucoup plus. Si quelque chose écrit dans une liste, rien d'autre ne doit y être lu ou écrit.

Dans .NET 4.0, nous avons des collections simultanées, qui sont facilement threadsafe et ne nécessitent pas de verrous.

Talljoe
la source
Cela a du sens, je vais certainement regarder les nouvelles collections Concurrent pour cela. Merci.
e36M3
11
Notez qu'il n'y a pas de ConcurrentListtype intégré . Il y a des sacs, des dictionnaires, des piles, des files d'attente, etc., mais pas de listes.
LukeH
11

Votre approche actuelle n'est pas thread-safe - je suggérerais d'éviter cela complètement - puisque vous effectuez essentiellement une transformation de données PLINQ pourrait être une meilleure approche (je sais que c'est un exemple simplifié, mais à la fin, vous projetez chaque transaction dans un autre état " " objet).

List<object> list = transactions.AsParallel()
                                .Select( tran => new object())
                                .ToList();
Verre brisé
la source
J'ai présenté un exemple trop simplifié pour souligner l'aspect de List.Add qui m'intéressait. My Parallel.Foreach en fait fera un bon travail et ne sera pas une simple transformation de données. Merci.
e36M3
4
les collections simultanées peuvent paralyser vos performances parallèles si elles sont utilisées sans nécessité - une autre chose que vous pouvez faire est d'utiliser un tableau de taille fixe et d'utiliser la Parallel.Foreachsurcharge qui prend en index - dans ce cas, chaque thread manipule une entrée de tableau différente et vous devriez être en sécurité.
BrokenGlass
6

Ce n'est pas une chose déraisonnable à demander. Il existe des cas où les méthodes qui peuvent causer des problèmes de sécurité des threads en combinaison avec d'autres méthodes sont sûres si elles sont la seule méthode appelée.

Cependant, ce n'est clairement pas un cas, si l'on considère le code affiché dans le réflecteur:

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

Même s'il EnsureCapacityétait en lui-même threadsafe (et ce n'est certainement pas le cas), le code ci-dessus ne sera clairement pas threadsafe, compte tenu de la possibilité d'appels simultanés à l'opérateur d'incrémentation provoquant des erreurs d'écriture.

Soit verrouiller, utiliser ConcurrentList, ou peut-être utiliser une file d'attente sans verrou comme lieu d'écriture de plusieurs threads, et la lecture de celle-ci - soit directement, soit en remplissant une liste avec elle - après avoir fait leur travail (je suppose que plusieurs écritures simultanées suivies d'une lecture à un seul thread est votre modèle ici, à en juger par votre question, sinon je ne peux pas voir comment la condition où Addest la seule méthode appelée pourrait être utile).

Jon Hanna
la source
6

Si vous souhaitez utiliser à List.addpartir de plusieurs threads et que vous ne vous souciez pas de l'ordre, vous n'avez probablement pas besoin de la capacité d'indexation d'un de Listtoute façon, et devez utiliser certaines des collections simultanées disponibles à la place.

Si vous ignorez ce conseil et ne faites que le faire add, vous pouvez rendre les addthreads sûrs mais dans un ordre imprévisible comme celui-ci:

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

...

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

Si vous acceptez cet ordre imprévisible, il est probable que, comme mentionné précédemment, vous n'ayez pas besoin d'une collection capable d'indexer comme dans someList[i].

tomsv
la source
5

Cela poserait des problèmes, car la liste est construite sur un tableau et n'est pas sûre pour les threads, vous pouvez obtenir une exception d'index hors limites ou certaines valeurs remplaçant d'autres valeurs, selon l'emplacement des threads. En gros, ne le faites pas.

Il y a plusieurs problèmes potentiels ... Ne le faites pas. Si vous avez besoin d'une collection thread-safe, utilisez un verrou ou l'une des collections System.Collections.Concurrent.

Linkgoron
la source
5

J'ai résolu mon problème en utilisant ConcurrentBag<T>au lieu de List<T>comme ceci:

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

Y a-t-il quelque chose de mal à ajouter simplement des éléments dans une liste si les threads n'effectuent jamais aucune autre opération sur la liste?

Réponse courte: oui.

Réponse longue: exécutez le programme ci-dessous.

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
la source
@royi exécutez-vous ceci sur une seule machine principale?
Bas Smit
Salut, je pense qu'il y a un problème avec cet exemple car il définit l'AutoResetEvent chaque fois qu'il trouve le nombre 1000. Parce qu'il peut traiter ces threads un peu quand il le souhaite, il peut atteindre 1000 avant d'atteindre 999 par exemple. Si vous ajoutez un Console.WriteLine dans la méthode AddTol, vous verrez que la numérotation n'est pas dans l'ordre.
Dave Walker
@dave, il définit l'événement quand i == 0
Bas Smit
2

Comme d'autres l'ont déjà dit, vous pouvez utiliser des collections simultanées à partir de l' System.Collections.Concurrentespace de noms. Si vous pouvez en utiliser un, c'est préférable.

Mais si vous voulez vraiment une liste qui est juste synchronisée, vous pouvez regarder la SynchronizedCollection<T>-Class in System.Collections.Generic.

Notez que vous deviez inclure l'assembly System.ServiceModel, ce qui est également la raison pour laquelle je ne l'aime pas tant. Mais parfois je l'utilise.

maf-doux
la source