Как реализовать потокобезопасный безошибочный обработчик событий в C #? - PullRequest
6 голосов
/ 29 июня 2011

проблемный фон

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

Это можно легко сделать с помощью такого кода:

public event EventHandler MyEvent;
public void RaiseEventSafely( object sender, EventArgs e )
{
    foreach(EventHandlerType handler in MyEvent.GetInvocationList())
        try {handler( sender, e );}catch{}
}


Общее, многопоточное, безошибочное решение

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

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

public class SafeEventHandler<EventType> where EventType:Delegate
{
    private object collection_lock = new object();
    private EventType event_handlers;

    public SafeEventHandler(){}

    public event EventType Handlers
    {
        add {lock(collection_lock){event_handlers += value;}}
        remove {lock(collection_lock){event_handlers -= value;}}
    }

    public void RaiseEventSafely( EventType event_delegate, object[] args )
    {
        lock (collection_lock)
            foreach (Delegate handler in event_delegate.GetInvocationList())
                try {handler.DynamicInvoke( args );}catch{}
    }
}


Ошибка компилятора с оператором + =, но два простых обходных пути

Одна проблема, с которой столкнулся, заключается в том, что строка "event_handlers + = value;" приводит к ошибке компилятора «Оператор + не может быть применен к типам EventType и EventType». Даже если EventType ограничен типом «Делегат», он не разрешит использовать оператор + =.

В качестве обходного пути я просто добавил ключевое слово event в "event_handlers", поэтому определение выглядит так: "private event EventType event_handlers;", и это прекрасно компилируется. Но я также подумал, что, поскольку ключевое слово «event» может генерировать код, чтобы справиться с этим, я тоже должен это сделать, поэтому в конечном итоге я изменил его на это, чтобы избежать неспособности компилятора распознать, что «+ =» ДОЛЖНО применяться к универсальный тип ограничен, чтобы быть делегатом. Закрытая переменная «event_handlers» теперь печатается как Delegate вместо общего EventType, а методы добавления / удаления следуют этому шаблону event_handlers = MulticastDelegate.Combine( event_handlers, value );


Окончательный код выглядит следующим образом:

public class SafeEventHandler<EventType> where EventType:Delegate
{
    private object collection_lock = new object();
    private Delegate event_handlers;

    public SafeEventHandler(){}

    public event EventType Handlers
    {
        add {lock(collection_lock){event_handlers = Delegate.Combine( event_handlers, value );}}
        remove {lock(collection_lock){event_handlers = Delegate.Remove( event_handlers, value );}}
    }

    public void RaiseEventSafely( EventType event_delegate, object[] args )
    {
        lock (collection_lock)
            foreach (Delegate handler in event_delegate.GetInvocationList())
                try {handler.DynamicInvoke( args );}catch{}
    }
}


Вопрос

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

Ответы [ 7 ]

18 голосов
/ 29 июня 2011

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

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

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

Я хочу игнорировать любые ошибки, возникающие в каждом отдельном обработчике.

Это супер опасная идея.Эти исключения говорят вам, что происходит что-то ужасное, а вы их игнорируете.

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

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

Затем я создаю объект блокировки и блокирую его в методах добавления и удаления открытого события, которые изменяют частную переменную-делегат с именем "event_handlers".

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

Нов этом сценарии этот код очень, очень, очень опасен:

    lock (collection_lock)
        foreach (Delegate handler in event_delegate.GetInvocationList())
            try {handler.DynamicInvoke( args );}catch{}

Давайте подумаем, что там не так.

Нить Альфа входит в блокировку коллекции.

Предположим, есть еще один ресурс, foo, который также контролируется другой блокировкой.Thread Beta входит в блокировку foo для получения необходимых ему данных.

Затем Beta-поток принимает эти данные и пытается войти в блокировку коллекции, поскольку он хочет использовать содержимое foo в обработчике событий.

Тема бета теперь ожидает потока Альфа.Альфа-поток теперь вызывает делегата, который решает, что он хочет получить доступ к foo.Итак, он ждет бета-потока, и теперь у нас тупик.

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

ОК, поэтому давайте предположим, что вы делаете это вместо этого:

    Delegate copy;
    lock (collection_lock)
        copy = event_delegate;
    foreach (Delegate handler in copy.GetInvocationList())
        try {handler.DynamicInvoke( args );}catch{}

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

Не совсем.Вы обменяли одну проблему на другую:

Поток Альфа берет блокировку, делает копию списка делегатов и покидает блокировку.

Поток Бета-версия берет блокировку, удаляет обработчик событияX из списка и уничтожает состояние, необходимое для предотвращения взаимоблокировки X.

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

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


Короче говоря, ваше решение является универсальным, но оно не поточно-ориентированное и не безошибочное. Скорее, это усугубляет проблемы с потоками, такие как взаимоблокировки, и отключает системы безопасности.

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

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

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

2 голосов
/ 29 июня 2011

Блокировка внутри RaiseEventSafely и ненужна, и опасна.

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

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

У вас есть пустой catch для exception.Это редко хорошая идея, поскольку она молча проглатывает исключение.Как минимум, вы должны зарегистрировать исключение.

Ваш общий параметр не начинается с T.Это немного сбивает с толку IMO.

where EventType:Delegate Я не думаю, что это компилируется.Delegate не является допустимым общим ограничением.По какой-то причине спецификация C # запрещает определенные типы как общее ограничение, и один из них - Delegate.(не знаю почему)

1 голос
/ 29 июня 2011

Ювал Леви (Juval Löwy) рассказывает об этом в своей книге «Программирование компонентов .NET».

http://books.google.com/books?id=m7E4la3JAVcC&lpg=PA129&pg=PA143#v=onepage&q&f=false

1 голос
/ 29 июня 2011

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

Более того, я не уверен, что следую коду в SafeEventHandler.RaiseEventSafely. Почему в качестве параметра указывается делегат события? Кажется, он не имеет отношения к полю event_handlers. Что касается безопасности потока, то после вызова GetInvocationList не имеет значения, была ли изменена исходная коллекция делегатов, поскольку возвращаемый массив не изменится.

Если вам нужно, я бы предложил сделать следующее:

class MyClass
    {
        event EventHandler myEvent;

        public event EventHandler MyEvent
        {
            add { this.myEvent += value.SwallowException(); }
            remove { this.myEvent -= value.SwallowException(); }
        }

        protected void OnMyEvent(EventArgs args)
        {
            var e = this.myEvent;
            if (e != null)
                e(this, args);
        }
    }

    public static class EventHandlerHelper
    {
        public static EventHandler SwallowException(this EventHandler handler)
        {
            return (s, args) =>
            {
                try
                {
                    handler(s, args);
                }
                catch { }
            };
        }
    }
1 голос
/ 29 июня 2011

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

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

Причина, по которой я это сделаю, заключается в том, что в противном случае вы рискуете выполнитьпоследствия и, возможно, тупики.Вы держите блокировку во время вызова чужого кода.Давайте назовем ваш внутренний замок Lock 'A'.Если один из обработчиков пытается получить закрытую блокировку «B», и в отдельном потоке кто-то пытается зарегистрировать обработчик, удерживая блокировку «B», тогда один поток удерживает блокировку «A» при попытке получить «B» идругой поток удерживает блокировку «B» при попытке получить блокировку «A».Deadlock.

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

Последний придирВы думаете, SafeEventHandler действительно описывает, что делает этот класс?Для меня это выглядит как регистратор событий и диспетчер.

1 голос
/ 29 июня 2011

Вы изучали классы PRISM EventAggregator или MVVMLight Messenger ? Оба этих класса отвечают всем вашим требованиям. Класс Messenger MVVMLight использует WeakReferences для предотвращения утечек памяти.

0 голосов
/ 29 июня 2011

Я учел все, что все говорили, и пришел к следующему коду:

public class SafeEvent<EventDelegateType> where EventDelegateType:class
{
    private object collection_lock = new object();
    private Delegate event_handlers;

    public SafeEvent()
    {
        if(!typeof(Delegate).IsAssignableFrom( typeof(EventDelegateType) ))
            throw new ArgumentException( "Generic parameter must be a delegate type." );
    }

    public Delegate Handlers
    {
        get
        {
            lock (collection_lock)
                return (Delegate)event_handlers.Clone();
        }
    }

    public void AddEventHandler( EventDelegateType handler )
    {
        lock(collection_lock)
            event_handlers = Delegate.Combine( event_handlers, handler as Delegate );
    }

    public void RemoveEventHandler( EventDelegateType handler )
    {
        lock(collection_lock)
            event_handlers = Delegate.Remove( event_handlers, handler as Delegate );
    }

    public void Raise( object[] args, out List<Exception> errors )
    {
        lock (collection_lock)
        {
            errors = null;
            foreach (Delegate handler in event_handlers.GetInvocationList())
            {
                try {handler.DynamicInvoke( args );}
                catch (Exception err)
                {
                    if (errors == null)
                        errors = new List<Exception>();
                    errors.Add( err );
                }
            }
        }
    }
}

Это обходит особую обработку делегатом делегата как недопустимого базового класса. Также нельзя указывать события как Делегат.

Вот как SafeEvent будет использоваться для создания события в классе:

private SafeEvent<SomeEventHandlerType> a_safe_event = new SafeEvent<SomeEventHandlerType>();
public event SomeEventHandlerType MyEvent
{
    add {a_safe_event.AddEventHandler( value );}
    remove {a_safe_event.RemoveEventHandler( value );}
}

А вот как будет вызываться событие и обрабатываться ошибки:

List<Exception> event_handler_errors;
a_safe_event.Raise( new object[] {event_type, disk}, out event_handler_errors );
//Report errors however you want; they should not have occurred; examine logged errors and fix your broken handlers!

Подводя итог, задача этого компонента состоит в том, чтобы публиковать события в списке подписчиков атомарным способом (то есть событие не будет повторно инициировано, и список вызовов не будет изменен во время выполнения списка вызовов). Блокировка возможна, но ее легко избежать, контролируя доступ к SafeEvent, потому что обработчик должен будет порождать поток, который вызывает один из открытых методов SafeEvent, и затем ждать в этом потоке. В любом другом сценарии другие потоки будут просто блокироваться, пока поток-владелец блокировки не снимет блокировку. Кроме того, хотя я вообще не верю в игнорирование ошибок, я также не верю, что этот компонент предназначен для интеллектуального управления ошибками подписчиков и вынесения суждения о серьезности таких ошибок, а не их выбрасывания и риска сбоя. приложение, оно сообщает об ошибках вызывающей стороне «Повышение», поскольку вызывающая сторона, вероятно, будет в лучшем положении для обработки таких ошибок. С учетом сказанного эти компоненты обеспечивают некоторую устойчивость к событиям, которых нет в системе событий C #.

Я думаю, что людей беспокоит то, что запуск других подписчиков после возникновения ошибки означает, что они работают в нестабильном контексте. Хотя это может быть правдой, это означает, что приложение на самом деле написано неправильно, как бы вы ни смотрели на него. Сбой не является лучшим решением, чем запуск кода, потому что запуск кода позволит сообщать об ошибках и позволять проявить все последствия ошибки, а это, в свою очередь, поможет инженерам быстро и полностью понять природу ошибки и ИСПРАВИТЬ ИХ КОД БЫСТРЕЕ.

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...