ОБНОВИТЬ

Начиная с 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аксессорах. Кроме того, существует проблема возможных взаимоблокировок, если блокировка удерживается во время срабатывания события.

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

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

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

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

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

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

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

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

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

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

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

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

TheEvent(this, EventArgs.Empty);

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

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

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

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() );

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

"Почему явно-нулевая проверка 'стандартного шаблона'?"

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

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

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

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

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

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, которую я опубликовал всего за день до того, как был задан этот вопрос (!)

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

Мне действительно понравилось это чтение - нет! Хотя мне это нужно для работы с функцией 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, установите флаг блокировки при его создании, чтобы при каждом вызове он блокировал все подписки и отмены подписки на него во время его выполнения?

Вывод ,

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

С C # 6 и выше, код может быть упрощен с помощью ?.оператора new, как в:

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

Вот документация MSDN.

Согласно Джеффри Рихтеру в книге 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);

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

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

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, похоже, не нравится, когда вы пытаетесь обновить представление после того, как его действие было отправлено в фоновый режим.

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

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

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

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

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

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

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

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

для однопоточных приложений вы исправляете это не проблема.

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

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

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

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

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

Я делаю это неправильно?

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

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

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

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

Честно говоря, я думаю, что класс «Делегат» не спасен. Слияние / переход к MulticastDelegate было огромной ошибкой, потому что оно фактически изменило (полезное) определение события с того, что происходит в один момент времени, на то, что происходит за промежуток времени. Такое изменение требует механизма синхронизации, который может логически свернуть его обратно в одно мгновение, но MulticastDelegate не имеет такого механизма. Синхронизация должна охватывать весь промежуток времени или момент, когда происходит событие, поэтому, когда приложение принимает синхронизированное решение начать обработку события, оно завершает его обработку полностью (транзакционно). С черным ящиком, который является гибридным классом MulticastDelegate / Delegate, это почти невозможно, поэтомупридерживайтесь использования одного подписчика и / или реализуйте свой собственный тип MulticastDelegate, у которого есть дескриптор синхронизации, который может быть удален во время использования / изменения цепочки обработчиков . Я рекомендую это, потому что альтернативой может быть избыточная реализация синхронизации / целостности транзакций во всех ваших обработчиках, что было бы нелепо / излишне сложно.

Пожалуйста, посмотрите здесь: http://www.danielfortunov.com/software/%24daniel_fortunovs_adventures_in_software_development/2009/04/23/net_event_invocation_thread_safety Это правильное решение и всегда должно использоваться вместо всех других обходных путей.

«Вы можете убедиться, что во внутреннем списке вызовов всегда есть хотя бы один член, инициализировав его анонимным методом« ничего не делать ». Поскольку ни одна внешняя сторона не может иметь ссылку на анонимный метод, никакая внешняя сторона не может удалить метод, поэтому делегат никогда не будет нулевым »- Программирование .NET Components, 2nd Edition, автор Juval Löwy

public static event EventHandler<EventArgs> PreInitializedEvent = delegate { };  

public static void OnPreInitializedEvent(EventArgs e)  
{  
    // No check required - event will never be null because  
    // we have subscribed an empty anonymous delegate which  
    // can never be unsubscribed. (But causes some overhead.)  
    PreInitializedEvent(null, e);  
}  

Я не верю, что вопрос ограничен типом события c #. Сняв это ограничение, почему бы не заново изобрести колесо и не сделать что-то подобное?

Поднимите поток событий безопасно - лучшая практика

• Возможность подписаться / отписаться от любой темы в то время как в рейсе (условие гонки удалено)
• Операторские перегрузки для + = и - = на уровне класса.
• Общий вызываемый абонентом делегат

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

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

/// <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();
    }

Тест

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

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 #.

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