Безопасность потоков C # с get / set - PullRequest
49 голосов
/ 03 февраля 2009

Это подробный вопрос для C #.

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

Object mLock = new Object();
MyObject property;
public MyObject MyProperty {
    get {
         return property;
    }
    set { 
         property = value; 
    }
}

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

Будет ли следующий код правильно блокировать данные?

Object mLock = new Object();
MyObject property;
public MyObject MyProperty {
    get {
         lock (mLock){
             return property;
         }
    }
    set { 
         lock (mLock){
              property = value; 
         }
    }
}

Под «правильно» я имею в виду, если я хочу позвонить

MyProperty.Field1 = 2;

или еще, будет ли поле заблокировано во время обновления? Является ли настройка, которая выполняется оператором equals, в области действия функции get, или сначала завершится функция get (и, следовательно, блокировка), а затем вызывается настройка, а затем - set, минуя таким образом замок?

Редактировать: Поскольку это, очевидно, не сработает, что будет? Нужно ли делать что-то вроде:

Object mLock = new Object();
MyObject property;
public MyObject MyProperty {
    get {
         MyObject tmp = null;
         lock (mLock){
             tmp = property.Clone();
         }
         return tmp;
    }
    set { 
         lock (mLock){
              property = value; 
         }
    }
}

, который более или менее просто гарантирует, что у меня есть только доступ к копии, что означает, что если бы у меня было два потока, вызывающих 'get' одновременно, каждый из них начинал бы с одинакового значения Field1 (справа) ?). Есть ли способ сделать блокировку чтения и записи для свойства, которое имеет смысл? Или я должен ограничиться блокировкой секций функций, а не самих данных?

Просто, чтобы этот пример имел смысл: MyObject - это драйвер устройства, который возвращает статус асинхронно. Я посылаю ему команды через последовательный порт, а затем устройство отвечает на эти команды в свое приятное время. Прямо сейчас у меня есть поток, который опрашивает его на предмет его статуса («Вы все еще там? Можете ли вы принимать команды?»), Поток, который ожидает ответов на последовательный порт («Только что получил строку состояния 2, все хорошо» ), а затем поток пользовательского интерфейса, который принимает другие команды («Пользователь хочет, чтобы вы сделали это».) и отправляет ответы от драйвера («Я только что сделал это, теперь обновите интерфейс с этим»). Вот почему я хочу заблокировать сам объект, а не поля объекта; это было бы огромное количество блокировок, a и b, не каждое устройство этого класса имеет одинаковое поведение, только общее поведение, поэтому мне пришлось бы кодировать множество отдельных диалогов, если бы я индивидуализировал блокировки.

Ответы [ 9 ]

39 голосов
/ 03 февраля 2009

Нет, ваш код не будет блокировать доступ к членам объекта, возвращенного из MyProperty. Он блокирует только MyProperty.

Ваш пример использования на самом деле представляет собой две операции, объединенные в одну, что примерно соответствует следующему:

// object is locked and then immediately released in the MyProperty getter
MyObject o = MyProperty;

// this assignment isn't covered by a lock
o.Field1 = 2;

// the MyProperty setter is never even called in this example

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

РЕДАКТИРОВАТЬ в ответ на дальнейшие детали в вопросе

Я все еще не уверен на 100%, чего вы пытаетесь достичь, но если вы просто хотите атомарный доступ к объекту, то не могли бы вы заблокировать вызывающий код для самого объекта?

// quick and dirty example
// there's almost certainly a better/cleaner way to do this
lock (MyProperty)
{
    // other threads can't lock the object while you're in here
    MyProperty.Field1 = 2;
    // do more stuff if you like, the object is all yours
}
// now the object is up-for-grabs again

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

14 голосов
/ 03 февраля 2009

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

objectRef.MyProperty += 1;

Гонка чтения-изменения-записи довольно очевидна, есть и худшие. Вы абсолютно ничего не можете сделать, чтобы сделать свое свойство поточно-безопасным, кроме как сделать его неизменным. Это ваш клиент, который должен иметь дело с головной болью. Ахиллесова пята параллельного программирования - быть вынужденным делегировать такую ​​ответственность программисту, который с наименьшей вероятностью понимает это правильно.

4 голосов
/ 03 февраля 2009

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

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

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

Как пример ...

public class Status
{
    private int _code;
    private DateTime _lastUpdate;
    private object _sync = new object(); // single lock for both fields

    public int Code
    {
        get { lock (_sync) { return _code; } }
        set
        {
            lock (_sync) {
                _code = value;
            }

            // Notify listeners
            EventHandler handler = Changed;
            if (handler != null) {
                handler(this, null);
            }
        }
    }

    public DateTime LastUpdate
    {
        get { lock (_sync) { return _lastUpdate; } }
        set { lock (_sync) { _lastUpdate = value; } }
    }

    public event EventHandler Changed;
}

Ваша ветка 'опрос' будет выглядеть примерно так:

Status status = new Status();
ManualResetEvent changedEvent = new ManualResetEvent(false);
Thread thread = new Thread(
    delegate() {
        status.Changed += delegate { changedEvent.Set(); };
        while (true) {
            changedEvent.WaitOne(Timeout.Infinite);
            int code = status.Code;
            DateTime lastUpdate = status.LastUpdate;
            changedEvent.Reset();
        }
    }
);
thread.Start();
2 голосов
/ 03 февраля 2009

Область блокировки в вашем примере находится в неверном месте - она ​​должна находиться в области видимости свойства класса 'MyObject', а не его контейнера.

Если класс объекта MyObject my просто используется для хранения данных, в которые один поток хочет записать, а другой (поток пользовательского интерфейса) для чтения, то вам, возможно, вообще не понадобится установщик, и он будет создан один раз.

Также рассмотрите, является ли размещение блокировок на уровне свойств уровнем записи степени детализации блокировки; если более одного свойства может быть записано для представления состояния транзакции (например, общее количество заказов и общий вес), то может быть лучше иметь блокировку на уровне MyObject (т.е. блокировку (myObject.SyncRoot) .. .)

1 голос
/ 03 февраля 2009

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

Код, который вы опубликовали, блокирует участника во время его чтения и записи. Если вы хотите обработать случай, когда значение обновляется, возможно, вам следует рассмотреть другие формы синхронизации, такие как events . (Проверьте авто / ручные версии). Затем вы можете сообщить своей ветке "опрос", что значение изменилось и оно готово к повторному чтению.

1 голос
/ 03 февраля 2009

В примере кода, который вы разместили, get никогда не формируется.

В более сложном примере:

MyProperty.Field1 = MyProperty.doSomething() + 2;

И, конечно, при условии, что вы сделали:

lock (mLock) 
{
    // stuff...
}

В doSomething() тогда все вызовы блокировки не будут достаточны для гарантии синхронизации по всему объекту. Как только функция doSomething() возвращается, блокировка теряется, затем добавление завершается, и затем происходит присвоение, которое снова блокируется.

Или, чтобы написать это по-другому, вы можете притвориться, будто блокировки не сделаны автоматически, и переписать это больше как «машинный код» с одной операцией на строку, и это станет очевидным:

lock (mLock) 
{
    val = doSomething()
}
val = val + 2
lock (mLock)
{
    MyProperty.Field1 = val
}
0 голосов
/ 19 февраля 2016

Разве блокировки C # не страдают теми же проблемами блокировки, что и другие языки?

Е.Г.

var someObj = -1;

// Thread 1

if (someObj = -1)
    lock(someObj)
        someObj = 42;

// Thread 2

if (someObj = -1)
    lock(someObj)
        someObj = 24;

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

// Threads 1 & 2

if (someObj = -1)
    lock(someObj)
        if(someObj = -1)
            someObj = {newValue};

Просто что-то иметь в виду.

0 голосов
/ 15 октября 2012

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

Я написал статью о неизменных модельных классах в C #, которая может быть интересна в этом контексте: http://rickyhelgesson.wordpress.com/2012/07/17/mutable-or-immutable-in-a-parallel-world/

0 голосов
/ 03 февраля 2009

В вашей отредактированной версии вы все еще не предоставляете многопоточный способ обновления MyObject. Любые изменения в свойствах объекта необходимо выполнить внутри синхронизированного / заблокированного блока.

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

...