События C # и безопасность потоков

Question

UPDATE

Начиная с C # 6, ответ на этот вопрос:

SomeEvent?.Invoke(this, e);

---

Я часто слышу / читаю следующий совет:

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

// Copy the event delegate before checking/calling
EventHandler copy = TheEvent;

if (copy != null)
    copy(this, EventArgs.Empty); // Call any handlers on the copied list

Обновлено . Читая об оптимизации, я подумал, что для этого также может потребоваться изменчивость члена события, но Джон Скит в своем ответе заявляет, что CLR не оптимизирует копию.

Но между тем, чтобы эта проблема даже возникла, другой поток должен был сделать что-то вроде этого:

// Better delist from event - don't want our handler called from now on:
otherObject.TheEvent -= OnTheEvent;
// Good, now we can be certain that OnTheEvent will not run...

Фактическая последовательность может быть такой:

// Copy the event delegate before checking/calling
EventHandler copy = TheEvent;

// Better delist from event - don't want our handler called from now on:
otherObject.TheEvent -= OnTheEvent;    
// Good, now we can be certain that OnTheEvent will not run...

if (copy != null)
    copy(this, EventArgs.Empty); // Call any handlers on the copied list

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

Так это Программирование грузового культа ? Кажется, так - многие люди должны предпринять этот шаг, чтобы защитить свой код от нескольких потоков, тогда как на самом деле мне кажется, что события требуют гораздо большего внимания, чем это, прежде чем их можно будет использовать как часть многопоточного дизайна. , Следовательно, люди, которые не проявляют такой дополнительной заботы, могут также проигнорировать этот совет - это просто не проблема для однопоточных программ, и фактически, учитывая отсутствие volatile в большинстве примеров кода в Интернете, совет может не иметь никакого эффекта вообще.

(И не намного ли проще просто присвоить пустую delegate { } в декларации члена, чтобы вам никогда не приходилось проверять null в первую очередь?)

Обновлено: В случае, если неясно, я понял намерение совета - избегать исключения нулевой ссылки при любых обстоятельствах. Моя точка зрения заключается в том, что это исключение с нулевой ссылкой может возникнуть только в том случае, если другой поток исключен из события, и единственная причина для этого состоит в том, чтобы гарантировать, что никакие дополнительные вызовы не будут получены через это событие, что явно НЕ достигается этим методом , Вы бы скрывали состояние гонки - было бы лучше раскрыть это! Это нулевое исключение помогает обнаружить злоупотребление вашим компонентом. Если вы хотите, чтобы ваш компонент был защищен от злоупотреблений, вы можете последовать примеру WPF - сохранить идентификатор потока в конструкторе и затем выдать исключение, если другой поток пытается напрямую взаимодействовать с вашим компонентом. Или же реализовать действительно потокобезопасный компонент (задача не из легких).

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

Обновление в ответ на сообщения Эрика Липперта в блоге:

Итак, есть одна важная вещь, которую я упустил в обработчиках событий: «обработчики событий должны быть устойчивыми перед вызовом даже после того, как событие было отписано», и, следовательно, поэтому нам нужно заботиться только о возможности делегата мероприятия null. Задокументировано ли это требование к обработчикам событий?

И так: «Существуют и другие способы решения этой проблемы; например, инициализация обработчика для получения пустого действия, которое никогда не удаляется. Но выполнение нулевой проверки является стандартным шаблоном».

Итак, оставшийся фрагмент моего вопроса: почему явно-нулевая проверка "стандартного шаблона"? Альтернатива, назначающая пустой делегат, требует добавления только = delegate {} к объявление события, и это устраняет эти маленькие груды вонючей церемонии из каждого места, где происходит событие. Было бы легко убедиться, что пустой делегат дешевый для создания экземпляра. Или я все еще что-то упускаю?

Конечно, должно быть, (как предположил Джон Скит) это всего лишь совет .NET 1.x, который не исчез, как это должно было быть в 2005 году?

Answer 1

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

Без модификатора volatile возможно, что локальная копия будет устаревшей, но это все. Это не вызовет NullReferenceException.

И да, конечно, есть условие гонки - но всегда будет. Предположим, мы просто изменили код на:

TheEvent(this, EventArgs.Empty);

Теперь предположим, что список вызовов для этого делегата содержит 1000 записей. Вполне возможно, что действие в начале списка будет выполнено до того, как другой поток отменит подписку обработчика в конце списка. Однако этот обработчик все равно будет выполнен, потому что это будет новый список. (Делегаты неизменны.) Насколько я понимаю, это неизбежно.

Использование пустого делегата, безусловно, позволяет избежать проверки недействительности, но не устраняет условия гонки. Это также не гарантирует, что вы всегда «видите» последнее значение переменной.

Answer 2

Я вижу, что многие люди идут к расширению этого метода ...

public static class Extensions   
{   
  public static void Raise<T>(this EventHandler<T> handler, 
    object sender, T args) where T : EventArgs   
  {   
    if (handler != null) handler(sender, args);   
  }   
}

Это дает вам лучший синтаксис для вызова события ...

MyEvent.Raise( this, new MyEventArgs() );

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

Answer 3

«Почему явно-нулевая проверка -« стандартный шаблон »?»

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

Если вы всегда подписываете пустой делегат на ваши события при их создании, будут некоторые накладные расходы:

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

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

Я провел небольшое поверхностное тестирование производительности, чтобы увидеть влияние подхода подписки-пустого-делегата, и вот мои результаты:

Executing 50000000 iterations . . .
OnNonThreadSafeEvent took:      432ms
OnClassicNullCheckedEvent took: 490ms
OnPreInitializedEvent took:     614ms <--
Subscribing an empty delegate to each event . . .
Executing 50000000 iterations . . .
OnNonThreadSafeEvent took:      674ms
OnClassicNullCheckedEvent took: 674ms
OnPreInitializedEvent took:     2041ms <--
Subscribing another empty delegate to each event . . .
Executing 50000000 iterations . . .
OnNonThreadSafeEvent took:      2011ms
OnClassicNullCheckedEvent took: 2061ms
OnPreInitializedEvent took:     2246ms <--
Done

Обратите внимание, что в случае нуля или одного подписчика (обычного для элементов управления пользовательского интерфейса, где событий много), событие, предварительно инициализированное с пустым делегатом, заметно медленнее (более 50 миллионов итераций ...)

Дополнительную информацию и исходный код см. В этом блоге . Поток безопасности событий .NET , который я опубликовал всего за день до того, как этот вопрос был задан (!)

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

Answer 4

Мне очень понравилось это чтение - нет! Хотя он мне нужен для работы с функцией C #, которая называется events!

Почему бы не исправить это в компиляторе? Я знаю, что есть люди из MS, которые читают эти посты, поэтому, пожалуйста, не обращайте на это внимания!

1 - Нулевая проблема ) Почему бы вообще не сделать события пустыми вместо пустых? Сколько строк кода будет сохранено для проверки на ноль или необходимости прикрепить = delegate {} к объявлению? Пусть компилятор обрабатывает пустой случай, IE ничего не делает! Если все это имеет значение для создателя события, они могут проверить .Empty и сделать все, что им захочется! В противном случае все пустые проверки / добавления делегатов - это хаки для решения проблемы!

Честно говоря, я устал от того, чтобы делать это с каждым событием - он же шаблонный код!

public event Action<thisClass, string> Some;
protected virtual void DoSomeEvent(string someValue)
{
  var e = Some; // avoid race condition here! 
  if(null != e) // avoid null condition here! 
     e(this, someValue);
}

2 - проблема состояния гонки ) Я прочитал сообщение Эрика в блоге, я согласен, что H (обработчик) должен обрабатывать, когда он разыменовывает себя, но нельзя ли сделать событие неизменяемым / безопасным для потока? IE, установите флаг блокировки на свое создание, чтобы при каждом вызове он блокировал все подписки и отмены подписки на него во время выполнения?

Заключение

Разве современные языки не должны решать такие проблемы для нас?

Answer 5

Согласно Джеффри Рихтеру в книге CLR через C # , правильный метод:

// Copy a reference to the delegate field now into a temporary field for thread safety
EventHandler<EventArgs> temp =
Interlocked.CompareExchange(ref NewMail, null, null);
// If any methods registered interest with our event, notify them
if (temp != null) temp(this, e);

Потому что это заставляет ссылочную копию. Для получения дополнительной информации см. Его раздел «Событие» в книге.

Answer 6

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

private readonly object eventMutex = new object();

private event EventHandler _onEvent = null;

public event EventHandler OnEvent
{
  add
  {
    lock(eventMutex)
    {
      _onEvent += value;
    }
  }

  remove
  {
    lock(eventMutex)
    {
      _onEvent -= value;
    }
  }

}

private void HandleEvent(EventArgs args)
{
  lock(eventMutex)
  {
    if (_onEvent != null)
      _onEvent(args);
  }
}

В настоящее время я в основном работаю с Mono для Android, и Android, похоже, не нравится, когда вы пытаетесь обновить представление после того, как его действие было отправлено в фоновый режим.

Answer 7

С C # 6 и выше, код можно упростить, используя новый .? operator как в:

TheEvent?.Invoke(this, EventArgs.Empty);

Ссылка: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/operators/null-conditional-operators

Answer 8

Так что я немного опоздал на вечеринку здесь. :)

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

Учитывая это, решение состоит в том, чтобы сказать: «Ну, нулевые подписчики представлены нулем». Затем просто выполните нулевую проверку перед выполнением дорогостоящей операции. Я полагаю, что другим способом сделать это было бы иметь свойство Count для типа Delegate, поэтому вы выполняете дорогостоящую операцию, только если myDelegate.Count> 0. Использование свойства Count - довольно приятный шаблон, который решает исходную проблему. разрешить оптимизацию, а также имеет приятное свойство возможности вызываться без возникновения исключения NullReferenceException.

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

Примечание: это чистое предположение. Я не связан с языками .NET или CLR.

Answer 9

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

Рассуждения людей, которым небезразлично нулевое эталонное исключение, а не состояние гонки, потребуют глубоких психологических исследований. Я думаю, что это как-то связано с тем, что исправить проблему нулевых ссылок гораздо проще. Как только это будет исправлено, они вешают большой код «Миссия выполнена» на свой код и расстегивают молнию на своем летном костюме.

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

Answer 10

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

Суть в том, что список вызовов можно изменять даже при возникновении события.

/// <summary>
/// Thread safe event invoker
/// </summary>
public sealed class ThreadSafeEventInvoker
{
    /// <summary>
    /// Dictionary of delegates
    /// </summary>
    readonly ConcurrentDictionary<Delegate, DelegateHolder> delegates = new ConcurrentDictionary<Delegate, DelegateHolder>();

    /// <summary>
    /// List of delegates to be called, we need it because it is relatevely easy to implement a loop with list
    /// modification inside of it
    /// </summary>
    readonly LinkedList<DelegateHolder> delegatesList = new LinkedList<DelegateHolder>();

    /// <summary>
    /// locker for delegates list
    /// </summary>
    private readonly ReaderWriterLockSlim listLocker = new ReaderWriterLockSlim();

    /// <summary>
    /// Add delegate to list
    /// </summary>
    /// <param name="value"></param>
    public void Add(Delegate value)
    {
        var holder = new DelegateHolder(value);
        if (!delegates.TryAdd(value, holder)) return;

        listLocker.EnterWriteLock();
        delegatesList.AddLast(holder);
        listLocker.ExitWriteLock();
    }

    /// <summary>
    /// Remove delegate from list
    /// </summary>
    /// <param name="value"></param>
    public void Remove(Delegate value)
    {
        DelegateHolder holder;
        if (!delegates.TryRemove(value, out holder)) return;

        Monitor.Enter(holder);
        holder.IsDeleted = true;
        Monitor.Exit(holder);
    }

    /// <summary>
    /// Raise an event
    /// </summary>
    /// <param name="args"></param>
    public void Raise(params object[] args)
    {
        DelegateHolder holder = null;

        try
        {
            // get root element
            listLocker.EnterReadLock();
            var cursor = delegatesList.First;
            listLocker.ExitReadLock();

            while (cursor != null)
            {
                // get its value and a next node
                listLocker.EnterReadLock();
                holder = cursor.Value;
                var next = cursor.Next;
                listLocker.ExitReadLock();

                // lock holder and invoke if it is not removed
                Monitor.Enter(holder);
                if (!holder.IsDeleted)
                    holder.Action.DynamicInvoke(args);
                else if (!holder.IsDeletedFromList)
                {
                    listLocker.EnterWriteLock();
                    delegatesList.Remove(cursor);
                    holder.IsDeletedFromList = true;
                    listLocker.ExitWriteLock();
                }
                Monitor.Exit(holder);

                cursor = next;
            }
        }
        catch
        {
            // clean up
            if (listLocker.IsReadLockHeld)
                listLocker.ExitReadLock();
            if (listLocker.IsWriteLockHeld)
                listLocker.ExitWriteLock();
            if (holder != null && Monitor.IsEntered(holder))
                Monitor.Exit(holder);

            throw;
        }
    }

    /// <summary>
    /// helper class
    /// </summary>
    class DelegateHolder
    {
        /// <summary>
        /// delegate to call
        /// </summary>
        public Delegate Action { get; private set; }

        /// <summary>
        /// flag shows if this delegate removed from list of calls
        /// </summary>
        public bool IsDeleted { get; set; }

        /// <summary>
        /// flag shows if this instance was removed from all lists
        /// </summary>
        public bool IsDeletedFromList { get; set; }

        /// <summary>
        /// Constuctor
        /// </summary>
        /// <param name="d"></param>
        public DelegateHolder(Delegate d)
        {
            Action = d;
        }
    }
}

И использование:

    private readonly ThreadSafeEventInvoker someEventWrapper = new ThreadSafeEventInvoker();
    public event Action SomeEvent
    {
        add { someEventWrapper.Add(value); }
        remove { someEventWrapper.Remove(value); }
    }

    public void RaiseSomeEvent()
    {
        someEventWrapper.Raise();
    }

Test

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

var objects = Enumerable.Range(0, 1000).Select(x => new Bar(foo)).ToList();
Thread.Sleep(10);
objects.ForEach(x => x.Dispose());

В конструкторе Bar (объект слушателя) я подписываюсь на SomeEvent (который реализован, как показано выше) и отписываюсь в Dispose:

    public Bar(Foo foo)
    {
        this.foo = foo;
        foo.SomeEvent += Handler;
    }

    public void Handler()
    {
        if (disposed)
            Console.WriteLine("Handler is called after object was disposed!");
    }

    public void Dispose()
    {
        foo.SomeEvent -= Handler;
        disposed = true;
    }

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

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

Если были условия гонки, я должен увидеть сообщение в консоли, но оно пустое. Но если я использую события clr как обычно, я вижу, что он полон предупреждающих сообщений. Итак, я могу заключить, что можно реализовать потокобезопасные события в c #.

Что вы думаете?