C#事件和线程安全

我经常听到/阅读以下build议:

在检查一个事件之前,一定要先制作一个事件的副本,然后将其null 。 这将消除线程潜在的问题,在检查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 

更新 :我从阅读优化思想,这可能也需要事件成员变动,但Jon Skeet在他的答案中指出,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在作者退订后运行,但他们只是退订,以避免发生。 当然,真正需要的是在addremove访问器中进行适当的同步的自定义事件实现。 此外,如果在事件触发时进行locking,则存在可能的死锁问题。

那么这个货物邪教编程呢? 看起来这样 – 很多人必须采取这一步来保护他们的代码免受多个线程的影响,但实际上,在我看来,事件需要比这更多的关注,然后才能用作multithreadingdevise的一部分。 因此,那些没有采取额外关怀的人可能会忽略这个build议 – 对于单线程程序来说这不是一个问题,事实上,由于大多数在线示例代码中没有volatile ,所以build议可能会有根本没有效果。

(在成员声明中分配空的delegate { }并不是一件简单的事情,所以你不需要首先检查null

更新:如果不清楚的话,我确实掌握了build议的意图 – 在所有情况下避免空引用exception。 我的意思是,这个特定的空引用exception只有在另一个线程从事件中退出时才会发生,唯一的原因就是确保不会再有任何进一步的调用通过这个事件来接收,这显然不能通过这种技术来实现。 你会隐瞒一个竞争条件 – 最好揭露它! 该空例外有助于检测到您的组件的滥用。 如果你想让你的组件免受滥用,你可以按照WPF的例子 – 在你的构造函数中存储线程ID,然后在另一个线程试图直接与你的组件交互时抛出一个exception。 否则实现一个真正的线程安全的组件(不是一件容易的事)。

所以我认为只是做这个抄袭/检查的习惯用法就是货真价实的编程,给你的代码添加混乱和噪音。 要实际防止其他线程需要更多的工作。

更新回复Eric Lippert的博客文章:

所以我对事件处理程序错过了一件重要的事情:“事件处理程序要求在事件被取消订阅之后被称为”强健的“,显然,因此我们只需要关心事件的可能性委托为null对事件处理程序的要求是否在任何地方logging?

所以:“还有其他方法可以解决这个问题,例如初始化处理程序,使其具有一个永远不会被删除的空操作,但是执行空操作是标准模式。

所以我的问题的剩余片段是, 为什么显式空检查“标准模式”? 另一种方法是分配空的代表,只需要将= delegate {}添加到事件声明中,这样就可以消除事件发生的每一个地方那些小小的臭味。 确保空委托实例化很容易。 还是我还想念什么?

肯定肯定是这样(正如Jon Skeet所说),这只是.NET 1.x的build议,并没有消失,就像2005年应该做的那样。

由于条件的限制,JIT不允许在第一部分中进行优化。 我知道这是前段时间作为一个幽灵而提出的,但这是无效的。 (我刚才跟Joe Duffy或Vance Morrison检查过,我不记得是哪一个。)

如果没有可变修饰符,则可能是所采用的本地副本将会过期,但仅此而已。 它不会导致NullReferenceException

是的,肯定有一个竞争条件 – 但总会有的。 假设我们只是将代码更改为:

 TheEvent(this, EventArgs.Empty); 

现在假设该委托的调用列表有1000个条目。 在另一个线程退出接近列表末尾的处理程序之前,清单开始处的动作完全可能会被执行。 但是,该处理程序仍然会被执行,因为它将成为一个新的列表。 (代表是不可改变的)。据我所知,这是不可避免的。

使用一个空的委托肯定避免了无效性检查,但不能解决竞争条件。 它也不能保证你总是“看到”variables的最新值。

我看到很多人正朝着这样做的扩展方法前进…

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

并且在方法调用时捕获本地副本。

“为什么显式 – 空检查'标准模式'?

我怀疑这可能是因为空检查是更高性能的原因。

如果您在创build事件时始终订阅空白的委托,那么会有一些开销:

  • 构build空的委托的成本。
  • 构build委托链来包含它的代价。
  • 每次事件发生时调用无意义委托的代价。

(请注意,UI控件通常有大量的事件,其中大部分事件从来没有订阅过。必须为每个事件创build一个虚拟用户然后调用它才可能是一个重大的性能问题。

我做了一些粗略的性能testing,看看subscribe-empty-delegate方法的影响,下面是我的结果:

 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 

请注意,对于零个或一个订阅者(UI控件常见的事件丰富的情况),使用空委托预先初始化的事件明显较慢(超过5000万次迭代…)

欲了解更多信息和源代码,请访问这个博客文章的.NET事件调用线程安全性 ,我刚刚发布的问题前一天发表(!)

(我的testing设置可能有缺陷,请随时下载源代码并自行检查,任何反馈都将非常感谢。)

我真的很喜欢这个阅读 – 不是! 即使我需要它使用称为事件的C#function!

为什么不在编译器中解决这个问题? 我知道有MS的人阅读这些post,所以请不要点燃这个!

1 – Null问题 )为什么不使事件成为空的而不是null呢? 多less行代码将被保存为空检查或不得不粘贴一个= delegate {}到声明? 让编译器处理Empty情况下,IE什么都不做! 如果这件事对创build者来说很重要,那么他们可以检查.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 – 竞赛条件问题 )我读过Eric的博客文章,我同意H(处理程序)应该在自引用时进行处理,但不能将该事件设置为不可变/线程安全的。 IE,在其创build时设置一个锁标志,以便每当它被调用时,它locking所有的订阅和取消订阅它的执行?

结论

现代语言是不是应该为我们解决这样的问题呢?

根据Jeffrey Richter在C#中的书CLR ,正确的方法是:

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

因为它强制参考副本。 有关更多信息,请参阅本书中的“活动”部分。

我一直在使用这种devise模式来确保事件处理程序在取消订阅后不被执行。 虽然我还没有尝试过任何性能分析,但它工作得很好。

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

我现在主要使用Android的Mono,而Android在尝试更新视图之后,似乎并不喜欢它。

这种做法不是强制执行一定的操作顺序。 实际上是避免一个空引用exception。

人们关注空缺的参照例外而不是竞争条件的理由需要一些深入的心理学研究。 我认为这与修复空引用问题更容易的事实有关。 一旦这个问题得到解决,他们就会在代码上挂上一个“完成任务”的大旗,并解开他们的飞行服。

注意:修正竞争条件可能涉及使用同步标志轨道是否应该运行处理程序

所以我来晚了一点 🙂

至于使用null而不是null对象模式来表示没有订户的事件,请考虑这种情况。 你需要调用一个事件,但是构造对象(EventArgs)是不平凡的,在一般情况下你的事件没有订阅者。 如果你能够优化你的代码来检查你是否有任何订阅者,那么在你承担构build参数和调用事件的处理工作之前,这将对你有好处。

考虑到这一点,一个解决scheme是说:“好吧,零订户是由空值表示的”。 然后在执行昂贵的操作之前,只需执行空检查。 我想这样做的另一种方法是在委托types上有一个Count属性,所以如果myDelegate.Count> 0,你只会执行昂贵的操作。使用Count属性是一个很好的模式,可以解决原来的问题允许优化,并且还具有可以被调用而不引起NullReferenceException的好的属性。

但请记住,由于代表是引用types,它们被允许为空。 也许根本没有什么好的方法来隐藏这个事实,并且只支持事件的空对象模式,所以替代scheme可能迫使开发人员检查null和零用户。 这比现在更丑。

注:这是纯粹的猜测。 我不涉及.NET语言或CLR。

对于单线程应用程序,你是correc这不是一个问题。

但是,如果您制作暴露事件的组件,则不能保证组件的使用者不会去执行multithreading,在这种情况下,您需要做好最坏的准备。

使用空的委托确实解决了这个问题,但是也会导致每次调用该事件的性能受到影响,并且可能会影响GC。

你是正确的,消费者为了取消订阅,为了这个事情发生,但如果他们超过了临时副本,那么考虑已经在传输的消息。

如果你不使用临时variables,并且不使用空的委托,而有人取消订阅,你会得到一个空引用exception,这是致命的,所以我认为这个代价是值得的。

我从来没有真正考虑过这是一个问题,因为我通常只是防止这种可能的线程在我的可重用组件上的静态方法(等)的坏处,我不做静态事件。

我做错了吗?

在施工时将所有活动连接起来,让他们独处。 Delegate类的devise不可能正确地处理任何其他的用法,我将在这篇文章的最后一段中解释。

首先,当你的事件处理程序必须已经对是否/如何回应通知做出同步的决定时,试图拦截事件通知没有任何意义。

任何可能被通知的事情都应该被通知。 如果您的事件处理程序正确地处理了通知(即,他们可以访问权威的应用程序状态并仅在适当的时候作出响应),那么在任何时候都可以通知他们并相信他们会正确响应。

不应该通知处理程序事件已经发生的唯一时间是事件实际上还没有发生! 所以,如果你不想要一个处理程序被通知,停止生成事件(即禁用控制或任何负责检测和事件首先存在)。

老实说,我认为代表class是无法挽回的。 合并/转换到一个MulticastDelegate是一个巨大的错误,因为它有效地改变了事件的(有用的)定义,从某个瞬间发生的事情到某个时间发生的事情。 这种改变需要一个同步机制,可以在逻辑上将其折叠回一个瞬间,但MulticastDelegate缺less任何这样的机制。 同步应该包含事件发生的整个时间或瞬间,以便一旦应用程序做出同步决定开始处理一个事件,它就完成处理(事务)。 使用MulticastDelegate / Delegate混合类的黑盒子,这几乎是不可能的,所以坚持使用单用户和/或实现你自己的一种MulticastDelegate,它有一个同步句柄,可以在处理器链被使用/修改 。 我build议这样做,因为替代方法是在所有的处理程序中冗余地实现同步/事务完整性,这将是荒谬/不必要的复杂。

Please take a look here: http://www.danielfortunov.com/software/%24daniel_fortunovs_adventures_in_software_development/2009/04/23/net_event_invocation_thread_safety This is the correct solution and should always be used instead of all other workarounds.

“You can ensure that the internal invocation list always has at least one member by initializing it with a do-nothing anonymous method. Because no external party can have a reference to the anonymous method, no external party can remove the method, so the delegate will never be null” — Programming .NET Components, 2nd Edition, by 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); } 

I don't believe the question is constrained to the c# "event" type. Removing that restriction, why not re-invent the wheel a bit and do something along these lines?

Raise event thread safely – best practice

  • Ability to sub/unsubscribe from any thread while within a raise (race condition removed)
  • Operator overloads for += and -= at the class level.
  • Generic caller-defined delegate

Thanks for a useful discussion. I was working on this problem recently and made the following class which is a bit slower, but allows to avoid callings to disposed objects.

The main point here is that invocation list can be modified even event is raised.

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

And the usage is:

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

testing

I tested it in the following manner. I have a thread which creates and destroys objects like this:

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

In a Bar (a listener object) constructor I subscribe to SomeEvent (which is implemented as shown above) and unsubscribe in 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; } 

Also I have couple of threads which raise event in a loop.

All these actions are performed simultaneously: many listeners are created and destroyed and event is being fired at the same time.

If there were a race conditions I should see a message in a console, but it is empty. But if I use clr events as usual I see it full of warning messages. So, I can conclude that it is possible to implement a thread safe events in c#.

你怎么看?

With C# 6 and above, code could be simplified using new .? operator as in TheEvent?.Invoke(this, EventArgs.Empty);

https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/operators/null-conditional-operators