Как заставить пользовательский универсальный список Thread Safe возвращать весь список в С#?

Я многопоточный нуб, и я пытаюсь написать собственный класс универсального списка, безопасный для потоков, на С# (.NET 3.5 SP1). Я прочитал Почему так сложно работать с потокобезопасными коллекциями?. Изучив требования класса, я думаю, что мне нужно только безопасно добавить в список и вернуть список . В примере показано почти все, что мне нужно, за исключением того, что в нем отсутствует метод return list, поэтому я написал свой собственный общедоступный метод, как показано ниже:

Обновление: на основе предложений я рассмотрел свои требования и, следовательно, упростил класс, как показано ниже:

public sealed class ThreadSafeList<T>
{
    private readonly IList<T> list = new List<T>();
    private readonly object lockable = new object();

    public void Add(T t)
    {
        lock (lockable)
        {
            list.Add(t);
        }
    }

    public IList<T> GetSnapshot()
    {
        IList<T> result;
        lock (lockable)
        {
            result = new List<T>(list);
        }
        return result;
    }
}

list generic-list c# thread-safety
person Jeff    schedule 24.02.2010    source источник
comment
Выглядит лучше. Последнее, что я хотел бы сделать, это сделать список, возвращаемый функцией Translate(), доступным только для чтения. Чтение содержимого в списке должно быть по своей сути операцией только для чтения... разрешая добавление списка, возвращенного из Translate(), вы оставляете дверь открытой для того, чтобы кто-то добавил в него, что может создать некоторые уродливые несоответствия. Я бы изменил «возврат результата»; to 'вернуть результат.AsReadOnly();'   -  person jrista    schedule 25.02.2010
comment
Ну, у меня все еще есть сомнения по поводу вашей конкретной реализации. ReadOnlyCollection - это просто оболочка... она не копирует список, который вы передаете своему конструктору, а просто предоставляет к нему доступ только для чтения. Таким образом, если вы пытаетесь перечислить список, возвращаемый Translate(), вы можете столкнуться с исключениями, если кто-то вызовет ThreadSafeList.Add() во время этого перечисления. Я по-прежнему рекомендую явно копировать список и выполнять return result.AsReadOnly(), чтобы этого не произошло. Он использует больше памяти, но не столкнется с проблемами потоковой передачи.   -  person jrista    schedule 25.02.2010
comment
Создание потоков сложно, и спасибо всем, кто внес свой вклад!   -  person Jeff    schedule 25.02.2010

Ответы (4)


arrow_upward
2
arrow_downward

Согласен с @jrista. Вам нужно решить проблему семантики, и почему она называется Translate()? Каково намерение?

A - текущий код - вернуть оболочку внутреннего списка, доступную только для чтения

return new ReadOnlyCollection<T>(list);

У вас все еще есть проблемы с потоками, если исходный список изменен, если другой поток выполняет итерацию по списку. Пока вы знаете об этом, это не является большой проблемой.

B - копия только для чтения.

return new List<T>(list).AsReadOnly();

В этом списке нет проблем с потоками, потому что ничто не изменяет новый список. Единственная ссылка содержится в оболочке ReadOnlyCollection<T>.

C - обычная (доступная для записи) копия

return new List<T>(list);

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

Имеет ли значение, если другой потребитель получит копию списка, а затем изменит свою копию? Нужно ли потребителям видеть изменения в списке? Вам просто нужен потокобезопасный перечислитель?

public IEnumerator<T> ThreadSafeEnumerator()
{
    List<T> copy;
    lock(lockable)
        copy = new List<T>(list);

    foreach (var value in copy)
        yield return value;
}
person Robert Paulson    schedule 25.02.2010
comment
@ Роберт, ответы на твои последние несколько вопросов - нет. Спасибо за ваши проницательные объяснения. Я изменил название метода на GetSnapshot() и считаю, что теперь оно лучше объясняет цель. - person Jeff; 25.02.2010
comment
@Джеффри круто. GetSnapshot() — гораздо лучшее имя. Молодец. - person Robert Paulson; 25.02.2010

arrow_upward
1
arrow_downward

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

person Orentet    schedule 24.02.2010
comment
Уточните, пожалуйста, не полагаться на подобные решения. Спасибо. - person Jeff; 25.02.2010
comment
Я полагаю, что он имеет в виду решения, которые выполняют внутреннюю синхронизацию потоков. Ваш метод Add возвращает ВНУТРЕННИЙ список, который не синхронизирован, что позволяет получателю этого списка делать все, что он хочет, когда захочет, с блокировкой или без нее. Даже если они заблокируются... это будет другой объект, на котором они зафиксируются, и синхронизация в этот момент невозможна. Вы должны оставить синхронизацию потоков для потребителя (ей), а не пытаться обрабатывать ее внутри. Написание согласованной параллельной коллекции с сохранением потоков — нетривиальная задача. - person jrista; 25.02.2010
comment
@jrista, на самом деле этот вопрос является продолжением моего предыдущего вопроса stackoverflow.com/questions/2322557, и я считаю, что этот класс ThreadSafeList на самом деле используется в потребительском??? (метод, который вызывает метод 1 и метод 2). - person Jeff; 25.02.2010
comment
Неважно, где используется ваш ThreadSafeList. Дело в том, что ваш метод Add полностью ошибочен с точки зрения многопоточности. Он вернул вызывающему объекту несинхронизированное внутреннее состояние. Если вы хотите, чтобы ваш ThreadSafeList действительно БЫЛ безопасным, вам нужно изменить то, что возвращается. Либо ничего не вернуть, либо индекс добавленной записи. Но не возвращайте ЛЮБОЕ внутреннее состояние... иначе ваша блокировка бесполезна. - person jrista; 25.02.2010
comment
@jrista, я думал, что то, что вы описываете, было исправлено. - person Jeff; 25.02.2010
comment
Ой, извините, я не перечитал ОП. Есть еще несколько других недостатков, которые я вижу... например, доступ к свойству IsReadOnly в существующем контексте блокировки, что может привести к взаимоблокировке (я не могу вспомнить, как .NET обрабатывает вложенные контексты блокировки.) Вы также не выбрасываете ReadOnlyException, если пытаетесь добавить, когда список доступен только для чтения, что для всех намерений и целей оставит список в неизвестном состоянии для вызывающего, что приведет к ArgumentOutOfRangeException, если вызывающий попытается получить элемент, который не был добавлен, и т. д. - person jrista; 25.02.2010

arrow_upward
1
arrow_downward

Метод Translate() выглядит правильно. Используя блокировку, вы не позволяете другим добавлять или иным образом изменять ваш список, пока вы находитесь в Translate/AddRange.

Я думаю, что может быть проблема с вашим свойством IsReadyOnly. Вы используете блокировку при чтении/записи свойства внутри. Но есть и публичный геттер, который не блокируется. Может случиться так, что поток 1 вызовет MarkAsReadOnly, в то время как второй поток все еще может получить false при просмотре IsReadOnly. Вместо этого я бы использовал обычное свойство и либо заблокировал геттер, либо использовал изменчивое логическое поле.

person stmax    schedule 24.02.2010

arrow_upward
1
arrow_downward

Вы можете использовать SynchronizedCollection.

http://msdn.microsoft.com/en-us/library/ms668265.aspx

person Jason Witty    schedule 30.08.2012