Потоковая безопасность при чтении свойства bool без блокировок

Question

Я пытался отследить чрезвычайно странную проблему, которая возникает очень редко и занимает много времени, чтобы проявиться. Этот шаблон кода, казалось, выделялся, и я хотел убедиться, что это потокобезопасно. Упрощенная форма шаблона здесь показывает класс TestClassManager, который управляет арендой для TestClass объектов. Объект TestClass будет сдан в аренду, использован и освобожден. После освобождения TestClass он не будет изменен / использован любым другим потоком в дальнейшем.

class Program
{
    public static void Main(string[] args)
    {
        var tasks = new List<Task>();
        var testClassManager = new TestClassManager();

        tasks.Add(Task.Factory.StartNew(() => TestersOperationLoop(testClassManager), TaskCreationOptions.LongRunning));
        tasks.Add(Task.Factory.StartNew(() => ClearTestersLoop(testClassManager), TaskCreationOptions.LongRunning));

        Task.WaitAll(tasks.ToArray());
    }

    public class TestClassManager
    {
        private readonly object _testerCollectionLock = new object();

        private readonly Dictionary<long, TestClass> _leasedTesters = new Dictionary<long, TestClass>();
        private readonly Dictionary<long, TestClass> _releasedTesters = new Dictionary<long, TestClass>();

        public TestClass LeaseTester()
        {
            lock (_testerCollectionLock)
            {
                var tester = new TestClass();

                _leasedTesters.Add(tester.Id, tester);
                _releasedTesters.Remove(tester.Id);

                return tester;
            }
        }

        public void ReleaseTester(long id)
        {
            lock (_testerCollectionLock)
            {
                var tester = _leasedTesters[id];

                _leasedTesters.Remove(tester.Id);
                _releasedTesters.Add(tester.Id, tester);
            }
        }

        public void Clear()
        {
            lock (_testerCollectionLock)
            {
                foreach (var tester in _releasedTesters)
                {
                    if (!tester.Value.IsChanged)
                    {
                        // I have not seen this exception throw ever, but can this happen?
                        throw new InvalidOperationException("Is this even possible!?!");
                    }
                }

                var clearCount = _releasedTesters.Count;

                _releasedTesters.Clear();
            }
        }
    }

    public class TestClass
    {
        private static long _count;

        private long _id;
        private bool _status;

        private readonly object _lockObject = new object();

        public TestClass()
        {
            Id = Interlocked.Increment(ref _count);
        }

        // reading status without the lock
        public bool IsChanged
        {
            get
            {
                return _status;
            }
        }

        public long Id { get => _id; set => _id = value; }

        public void SetStatusToTrue()
        {
            lock (_lockObject)
            {
                _status = true;
            }
        }
    }

    public static void TestersOperationLoop(TestClassManager testClassManager)
    {
        while (true)
        {
            var tester = testClassManager.LeaseTester();

            tester.SetStatusToTrue();
            testClassManager.ReleaseTester(tester.Id);
        }
    }

    public static void ClearTestersLoop(TestClassManager testClassManager)
    {
        while (true)
        {
            testClassManager.Clear();
        }
    }
}

Является ли проверка свойства TestClass.IsChanged изнутри TestClassManager.Clear метод потокобезопасен? Я никогда не вижу InvalidOperationException, но возможно ли это? Если это так, это могло бы объяснить мою проблему.

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

Спасибо!

Answer 1

TLDR: Ваш код является поточно-ориентированным. Вы правы, опасаясь, что чтение поля _status через свойство IsChanged может привести к устареванию значений; однако этого не произойдет в вашем методе Clear, поскольку другие существующие операторы lock уже защищают от этого.

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

Согласованность памяти является более эзотерической c topi c и имеет дело с порядком, в котором записи в ячейки памяти один поток может отображаться для чтения из тех же областей памяти другими потоками. Операции с памятью могут выглядеть переупорядоченными между потоками из соображений производительности, таких как лучшее использование локальных кэшей. Без синхронизации потоки могут считывать устаревшие значения областей памяти, которые были обновлены другими потоками - запись, по-видимому, была выполнена после чтения с точки зрения потока читателя. Чтобы избежать такого переупорядочения, программист может ввести в свой код барьеры памяти , что предотвратит перемещение операций с памятью. На практике такие барьеры памяти обычно генерируются неявно только в результате других операций с потоками. Например, оператор lock создает барьер памяти как при входе, так и при выходе.

Однако создание барьеров памяти с помощью оператора lock является только побочным эффектом, поскольку основная цель lock для обеспечения взаимного исключения. Это может привести к путанице, когда программисты сталкиваются с другими ситуациями, когда взаимное исключение также достигается, но барьеры памяти не создаются неявно. Одной из таких ситуаций является чтение и запись полей шириной до 32 или 64 бит, которые гарантированно будут иметь атомы c на архитектурах такой ширины. Чтение или запись логического значения по своей сути поточно-ориентированно - вам никогда не понадобится lock для принудительного взаимного исключения при этом, поскольку никакой другой параллельный поток не может «испортить» значение. Однако чтение или запись логического значения без lock означает, что не создаются барьеры памяти, поэтому могут возникнуть устаревшие значения.

Давайте применим это обсуждение к урезанной версии вашего кода:

// Thread A:
_testerStatus = true;                  // tester.SetStatusToTrue();
_testerReleased = true;                // testClassManager.ReleaseTester(tester.Id);

// Thread B:
if (_testerReleased)                               // foreach (var tester in _releasedTesters)
    Console.WriteLine($"Status: {_testerStatus}"); //     if (!tester.Value.IsChanged)

Возможно ли для вышеуказанной программы выводить false? В этом случае да. Это пример classi c, обсуждаемый в разделе Неблокирующая синхронизация (рекомендуемое чтение). Ошибка может быть исправлена ​​путем добавления барьеров памяти (явно, как показано ниже, или неявно, как описано далее ниже):

// Thread A:
_testerStatus = true;                  // tester.SetStatusToTrue();
Thread.MemoryBarrier();                // Barrier 1
_testerReleased = true;                // testClassManager.ReleaseTester(tester.Id);
Thread.MemoryBarrier();                // Barrier 2

// Thread B:
Thread.MemoryBarrier();                            // Barrier 3
if (_testerReleased)                               // foreach (var tester in _releasedTesters)
{
    Thread.MemoryBarrier();                        //     Barrier 4
    Console.WriteLine($"Status: {_testerStatus}"); //     if (!tester.Value.IsChanged)
}

В вашем коде ваш метод ReleaseTester имеет внешний lock, который неявно генерирует эквивалент барьеров 1 и 2 выше. Точно так же ваш Clear метод также имеет внешний lock, поэтому он генерирует эквивалент Barrier 3. Ваш код не генерирует эквивалент Barrier 4; однако я считаю, что этот барьер в вашем случае не нужен, поскольку взаимное исключение, установленное lock, влечет за собой то, что барьер 2 должен быть выполнен до барьера 3, если тестер был освобожден.

Answer 2

Кажется, проблема в том, что в вашем TestersOperationLoop вы LeaseTester, который позволяет ClearTestersLoop из другого потока немедленно Clear недавно сдать в аренду тест. Поскольку в этот момент вы еще не вызвали SetStatusToTrue, проверка в методе Clear не будет выполнена, и будет сгенерировано исключение.

  var tester = testClassManager.LeaseTester();

  // Imagine that testClassManager.Clear(); gets called from the second thread at this point.
  //
  // Since _testerCollectionLock from testClassManager.LeaseTester() is already released, the Clear method from other thread can run as it can acquire the lock now.
  // This thread keeps running, and races to start executing the next line which would set the status to true and prevent the other thread from throwing exception.
  // But it is a nature of race conditions that there is a single winner... and in this case it is the other thread; checking for status which is not yet changed to true and thus throwing the exception :-(

  tester.SetStatusToTrue();
  testClassManager.ReLeaseTester(tester.Id);

Самым простым исправлением в этом случае будет установка состояние TestClas внутри метода LeaseTester в блоке блокировки.

public TestClass LeaseTester()
{
  lock (_testerCollectionLock)
  {
    var tester = new TestClass();

    tester.SetStatusToTrue();

    _leasedTesters.Add(tester.Id, tester);
    _releasedTesters.Remove(tester.Id);

    return tester;
  }
}

Answer 3

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

Трудно рекомендовать исправление, не видя фактического использования. Попробуйте посмотреть, какие значения вы изменяете и читаете, и убедитесь, что они выполняются только в пределах одной блокировки. Один из вариантов - сделать объект блокировки TestManager общедоступным или добавить объект общей блокировки во все экземпляры TestClass.