Является ли этот неизменяемый объект безопасным? - PullRequest
Новая
       29

Является ли этот неизменяемый объект безопасным?

0 голосов
/ 02 октября 2019

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

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


    public class Tenants : ITenants
    {
        private readonly string url = "someurl";
        private readonly IHttpClientFactory httpClientFactory;

        private ConfigParser parser;

        public Tenants(IHttpClientFactory httpClientFactory)
        {
            this.httpClientFactory = httpClientFactory;
        }

        public async Task Refresh()
        {
            TConfig data = await ConfigLoader.GetData(httpClientFactory.CreateClient(), url);
            parser = new ConfigParser(data);
        }

        public async Task<TConfig> GetSettings(string name)
        {
            if (parser == null)
                await Refresh();

            return parser.GetSettings(name);

        }
    }

    public class ConfigParser
    {

        private readonly ImmutableDictionary<string, TConfig> configs;

        public ConfigParser(TConfig[] configs)
        {

            this.configs = configs.ToImmutableDictionary(s => s.name, v => v);
        }


        public TConfig GetSettings(string name)
        {
            if (!configs.ContainsKey(name))
            {
                return null;
            }

            return configs[name];
        }

    }

Класс Tenants будет введен какSingleton в другие классы через DI IOC.

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

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

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

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

1 Ответ

1 голос
/ 02 октября 2019

РЕДАКТИРОВАНИЕ:

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

Неизменяемость означает, что экземпляр объекта можетне может быть изменен (его внутреннее или внешнее состояние не может быть изменено).

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

Неизменяемые объекты являются поточно-ориентированными, но что-то не обязательно должно быть неизменным, чтобы быть поточно-ориентированным.

Ваш класс Tenants не является ни неизменным, ни поточно-ориентированным, потому что:

  1. Это внутреннее состояние может измениться после создания экземпляра.
  2. Он содержит непредвиденное поведение, когда запрос на получение конфигурации выполняется дважды, тогда как это должно происходить только один раз.

Если вы прочитаете мой ответ ниже, вы можете определить, что если вы не противce (которым вы не должны быть): вам не нужно ничего делать, но вы можете добавить ключевое слово volatile в поле parser, чтобы предотвратить сценарии SOME , но определенно не все.

Вы не видите никаких блокировок в неизменяемых объектах, потому что в состоянии объекта не происходит запись.

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

ConfigParser Кажетсячтобы быть потокобезопасным, Tenants определенно не является.

Ваш класс Tenants также не является неизменяемым, поскольку он предоставляет метод, который изменяет состояние класса (как GetSettings, так и Refresh методов).

Если 2 потока вызывают GetSettings одновременно, когда parser равен null, будет сделано 2 запроса на получение ConfigParser. Вы можете быть в порядке с этим, но это плохая практика, а также означает, что метод не является потокобезопасным.

Если у вас все в порядке с выполнением запроса дважды, вы можете использовать volatile here:

Ключевое слово volatile указывает, что поле может быть изменено несколькими потоками, которые выполняются одновременно. Компилятор, система времени выполнения и даже аппаратное обеспечение могут перестраивать операции чтения и записи в области памяти по соображениям производительности. Поля, которые объявлены как изменчивые, не подлежат этой оптимизации. Добавление модификатора volatile гарантирует, что все потоки будут наблюдать записи volatile, выполняемые любым другим потоком в том порядке, в котором они были выполнены.

Volatile будет препятствовать тому, чтобы потоки имели устаревшие значения. Это означает, что вы можете предотвратить некоторые дополнительных запросов (от потоков, которые по-прежнему считают parser нулевым), но это не будет полностью препятствовать выполнению метода или инструкции несколько раз в одно и то же время. ,

В этой ситуации вам необходимо lock:

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

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

К сожалению, вы не можете использовать await внутри lock.

. Что вы хотите сделать:

  • Если необходимо вызвать Refresh:
    • Если другой поток уже работает с Refresh
      • Подождите, пока другой поток завершит работу, и не вызывайте Refresh
      • Продолжите с результатом из другого потока
    • , если на Refresh
      • Вызовите метод Refresh

Я написал для этого библиотеку под названием TaskSynchronizer . Вы можете использовать это для создания истинной поточно-безопасной версии вашего класса Tenants.

Пример: 

public static TaskSynchronizer Synchronizer = new TaskSynchronizer();

public static async Task DoWork()
{
    await Task.Delay(100); // Some heavy work.
    Console.WriteLine("Work done!");
}

public static async Task WorkRequested()
{
    using (Synchronizer.Acquire(DoWork, out var task)) // Synchronize the call to work.
    {
        await task;
    }
}

static void Main(string[] args)
{
    var tasks = new List<Task>();
    for (var i = 0; i < 2; i++)
    {
        tasks.Add(WorkRequested());
    }
    Task.WaitAll(tasks.ToArray());
}

выведет: 

Work done!

EG: TheМетод async DoWork вызывается только один раз, даже если он вызывался дважды одновременно.

...