Этот пример потока безопасен?

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

public sealed class DataStore
{
    public static DataStore Instance { get { return _instance; } }
    public Dictionary<Foo, Bar> FooBar { get; private set; }

    static DataStore() { }
    private DataStore() { }

    public void Refresh() {
        FooBar = GetFooBarFromDB();
    }

    private static readonly DataStore _instance = new DataStore();
}

Мой вопрос по сути, это безопасно Refresh() в то время как другие потоки могут получать доступ FooBar? Нужно ли использовать блокировки, или мои операции получения и настройки являются атомарными? Нужно ли явно декларировать volatile поля для резервного копирования моих свойств?

PS Если кто-то может придумать более описательный заголовок для этого вопроса, я бы с радостью приветствовал это.

Редактировать: Исправлен мой пример для исправления явно неатомарного кода.

4 ответа

Решение

Да, в таких случаях вам нужна явная синхронизация, потому что другой поток может получить FooBar и начните читать, прежде чем закончите писать.

Если вы сделаете это, однако,

public void Refresh() {
    var tmp = new Dictionary<Foo, Bar>();
    // Fill out FooBar from DB
    FooBar = tmp;
}

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

Конечно, здесь есть неявное предположение, что за пределами Refresh метод.

РЕДАКТИРОВАТЬ: Вы также должны переключиться с автоматически внедренного свойства на вручную реализованное свойство с переменной поддержки, объявленной с volatile модификатор.

Ваш пример не является потокобезопасным. Словарь не является потокобезопасным классом, и любой поток может читать его во время выполнения обновления. Вы можете либо положить lock вокруг или использовать один из потоковобезопасных классов, таких как ConcurrentDictionary,

Что ж, мы согласны с тем, что ваш текущий код не является потокобезопасным. Итак, вы должны использовать функции синхронизации, потому что FooBar это ваш критический раздел.

Если вы позволите этому быть public, вы ожидаете, что люди за пределами DataStore класс будет действовать соответственно. Однако это плохое дизайнерское решение.

Итак, я бы посоветовал вам обернуть все в ваш текущий класс примерно так: как лучше всего реализовать поточно-безопасный словарь?

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

Вы можете легко получить один поток, добавляющий в коллекцию, в то время как другой перебирает его.

РЕДАКТИРОВАТЬ То, что я говорю... никогда не выставляйте статический словарь или любой другой тип коллекции. Всегда используйте параллельную версию

Другие вопросы по тегам