События C# и безопасность потоков
ОБНОВИТЬ
Начиная с 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 году?
15 ответов
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 и выше, код может быть упрощен с помощью нового .? operator как в TheEvent?.Invoke(this, EventArgs.Empty);
Согласно Джеффри Рихтеру в книге 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.
Вы правы в том, что пользователь пытается отменить подписку, чтобы это произошло, но если он прошел мимо временной копии, то рассмотрите сообщение, уже находящееся в пути.
Если вы не используете временную переменную и не используете пустой делегат, и кто-то отписывается, вы получаете исключение нулевой ссылки, которое является фатальным, поэтому я думаю, что цена того стоит.
Пожалуйста, посмотрите здесь: 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#. Сняв это ограничение, почему бы не заново изобрести колесо и не сделать что-то подобное?
Поднимите ветку событий безопасно - лучшая практика
- Возможность подписаться / отписаться от любой темы, находясь в пределах рейза (условие гонки удалено)
- Операторские перегрузки для += и -= на уровне класса.
- Общий вызываемый абонентом делегат
Я никогда не думал, что это большая проблема, потому что я, как правило, защищаю только от такого потенциального нарушения потоков в статических методах (и т. Д.) На моих повторно используемых компонентах, и я не создаю статические события.
Я делаю это неправильно?
Проведите все ваши мероприятия на стройке и оставьте их в покое. Дизайн класса Delegate не может правильно обрабатывать любое другое использование, как я объясню в последнем параграфе этого поста.
Прежде всего, нет смысла пытаться перехватить уведомление о событии, когда ваши обработчики событий уже должны принять синхронизированное решение о том, следует ли / как отвечать на уведомление.
Все, что может быть уведомлено, должно быть уведомлено. Если ваши обработчики событий обрабатывают уведомления надлежащим образом (т.е. они имеют доступ к официальному состоянию приложения и отвечают только в случае необходимости), тогда можно будет уведомить их в любое время и поверить, что они ответят правильно.
Единственный раз, когда обработчик не должен быть уведомлен о том, что произошло событие, - это если событие фактически не произошло! Поэтому, если вы не хотите, чтобы обработчик уведомлялся, прекратите генерировать события (то есть отключите элемент управления или что-то еще, что в первую очередь отвечает за обнаружение и реализацию события).
Честно говоря, я думаю, что класс "Делегат" не спасен. Слияние / переход к MulticastDelegate было огромной ошибкой, потому что оно фактически изменило (полезное) определение события с того, что происходит в один момент времени, на то, что происходит за промежуток времени. Такое изменение требует механизма синхронизации, который может логически свернуть его обратно в одно мгновение, но MulticastDelegate не имеет такого механизма. Синхронизация должна охватывать весь промежуток времени или момент, когда происходит событие, поэтому, когда приложение принимает синхронизированное решение начать обработку события, оно завершает его обработку полностью (транзакционно). С черным ящиком, который является гибридным классом MulticastDelegate/Delegate, это практически невозможно, поэтому придерживайтесь использования одного подписчика и / или реализуйте свой собственный тип MulticastDelegate, у которого есть дескриптор синхронизации, который может быть удален, пока цепочка обработчика используется / модифицируется. Я рекомендую это, потому что альтернативой может быть избыточная реализация синхронизации / целостности транзакций во всех ваших обработчиках, что было бы нелепо / излишне сложно.
Спасибо за полезное обсуждение. Недавно я работал над этой проблемой и создал следующий класс, который немного медленнее, но позволяет избежать вызовов удаленных объектов.
Суть в том, что список вызовов можно изменить даже при возникновении события.
/// <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#.
Как вы думаете?