Список.Добавить () безопасность резьбы


Я понимаю, что в целом список не является потокобезопасным, однако есть ли что-то не так с простым добавлением элементов в список, если потоки никогда не выполняют никаких других операций в списке (например, обход его)?

пример:

List<object> list = new List<object>();
Parallel.ForEach(transactions, tran =>
{
    list.Add(new object());
});
9 69

9 ответов:

за кулисами происходит много вещей, включая перераспределение буферов и копирование элементов. Этот код будет представлять опасность. Очень просто, нет атомарных операций при добавлении в список, по крайней мере, свойство "длина" должно быть обновлено, и элемент должен быть помещен в нужное место, и (если есть отдельная переменная) индекс должен быть обновлен. Несколько потоков могут топтать друг друга. И если рост требуется, то есть намного больше происходит. Если что-то это запись в список ничего другого не должно быть чтение или запись в него.

в .NET 4.0 у нас есть параллельные коллекции, которые удобно threadsafe и не требуют блокировки.

ваш текущий подход не является потокобезопасным - я бы предложил избегать этого вообще - так как вы в основном делаете преобразование данных PLINQ может быть лучшим подходом (я знаю, что это упрощенный пример, но в конце концов вы проецируете каждую транзакцию в другой объект "состояния").

List<object> list = transactions.AsParallel()
                                .Select( tran => new object())
                                .ToList();

Это вызовет проблемы, так как список построен над массивом и не является потокобезопасным, вы можете получить исключение index out of bounds или некоторые значения, переопределяющие другие значения, в зависимости от того, где находятся потоки. В принципе, не делайте этого.

есть несколько потенциальных проблем... Если вам нужна потокобезопасная коллекция, используйте либо блокировку, либо одну из систем.Коллекции.Параллельные коллекции.

это не безрассудная вещь, чтобы спросить. Там are случаи, когда методы, которые могут вызвать проблемы безопасности потока в сочетании с другими методами, безопасны, если они являются единственным вызываемым методом.

однако это явно не тот случай, когда вы рассматриваете код, показанный в reflector:

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

даже если EnsureCapacity по себе не ориентирована на многопотоковое исполнение (и это, безусловно, не является), приведенный выше код, очевидно, не собирается быть ориентирован на многопотоковое исполнение, учитывая возможность одновременных вызовов оператора инкремента, вызывающих неправильные записи.

либо заблокировать, использовать ConcurrentList, или, возможно, использовать lock-free queue в качестве места записи нескольких потоков, а чтение из него-либо напрямую, либо путем заполнения списка с ним - после того, как они выполнили свою работу (я предполагаю, что несколько одновременных записей с последующим однопоточным чтением-это ваш шаблон здесь, судя по вашему вопросу, поскольку в противном случае я не вижу, как условие where Add это единственный метод, который может быть использован).

если вы хотите использовать List.add из нескольких потоков и не заботятся о заказе, то вам, вероятно, не нужна способность индексирования List в любом случае, и следует использовать некоторые из доступных параллельных коллекций вместо этого.

если вы игнорируете этот совет и только add, вы могли бы сделать add потокобезопасно, но в непредсказуемом порядке, как это:

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

...

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

если вы принимаете этот непредсказуемый заказ, скорее всего, вы, как упоминалось ранее, не делаете нужна коллекция, которая может делать индексацию как в someList[i].

есть ли что-то неправильное в простом добавлении элементов в список, если потоки никогда не выполняют никаких других операций в списке?

короткий ответ: да.

длинный ответ: запустите программу ниже.

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();
    }
}

как уже говорили другие, вы можете использовать параллельные коллекции из System.Collections.Concurrent пространство имен. Если вы можете использовать один из них, это предпочтительнее.

но если вы действительно хотите список, который просто синхронизируется, вы можете посмотреть на SynchronizedCollection<T>-класс System.Collections.Generic.

обратите внимание, что вы должны включать систему.Сборка ServiceModel, которая также является причиной того, что мне это так не нравится. Но иногда я им пользуюсь.

даже добавление элементов на разных потоках не является потокобезопасным.

в C# 4.0 есть параллельные коллекции (см. http://jiezhu0815.blogspot.com/2010/08/c-40-feature-1-concurrent-collections.html).

Я решил свою проблему с помощью:

ConcurrentBag<T> 

вместо

IList<T>

https://docs.microsoft.com/dotnet/api/system.collections.concurrent.concurrentbag-1?view=netframework-4.7.2