我经常听到/读到以下建议:

在检查事件是否为空并触发它之前,始终要对事件进行复制。这将消除线程的一个潜在问题,即事件在你检查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

更新:我认为从阅读优化,这可能也需要事件成员是volatile,但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运行后,作者已经取消订阅,但他们只是取消订阅专门避免这种情况发生。当然,真正需要的是在添加和删除访问器中具有适当同步的自定义事件实现。此外,如果在触发事件时持有锁,则可能存在死锁的问题。

So is this Cargo Cult Programming? It seems that way - a lot of people must be taking this step to protect their code from multiple threads, when in reality it seems to me that events require much more care than this before they can be used as part of a multi-threaded design. Consequently, people who are not taking that additional care might as well ignore this advice - it simply isn't an issue for single-threaded programs, and in fact, given the absence of volatile in most online example code, the advice may be having no effect at all.

(在成员声明中分配空委托{}不是更简单吗,这样你就永远不需要在第一时间检查null ?)

Updated: In case it wasn't clear, I did grasp the intention of the advice - to avoid a null reference exception under all circumstances. My point is that this particular null reference exception can only occur if another thread is delisting from the event, and the only reason for doing that is to ensure that no further calls will be received via that event, which clearly is NOT achieved by this technique. You'd be concealing a race condition - it would be better to reveal it! That null exception helps to detect an abuse of your component. If you want your component to be protected from abuse, you could follow the example of WPF - store the thread ID in your constructor and then throw an exception if another thread tries to interact directly with your component. Or else implement a truly thread-safe component (not an easy task).

所以我认为,仅仅做这种复制/检查的习惯是盲目的编程,会给你的代码增加混乱和噪音。要真正防范其他线程,需要做更多的工作。

针对Eric Lippert博客文章的更新:

因此,关于事件处理程序,我错过了一个主要的事情:“事件处理程序必须健壮,即使在事件被取消订阅后也需要被调用”,显然,因此我们只需要关心事件委托为空的可能性。对事件处理程序的需求在任何地方都有记录吗?

所以:“有其他方法可以解决这个问题;例如,初始化处理程序以拥有一个永远不会删除的空操作。但做空检查是标准模式。”

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

当然,就像Jon Skeet所建议的那样,这只是。net 1。X条没有消失的建议,就像它在2005年应该消失的那样?


更新

从c# 6开始,这个问题的答案是:

SomeEvent?.Invoke(this, e);

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

但是,如果您正在制作一个公开事件的组件,则无法保证组件的使用者不会采用多线程,在这种情况下,您需要做好最坏的准备。

使用空委托确实解决了这个问题,但也会在每次调用事件时导致性能下降,并可能涉及GC。

您是对的,消费者试图取消订阅才会发生这种情况,但如果他们通过了临时副本,那么就认为消息已经在传输中。

如果你不使用临时变量,也不使用空委托,有人取消订阅,你会得到一个空引用异常,这是致命的,所以我认为代价是值得的。


由于条件的原因,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() );

并且还消除了本地副本,因为它是在方法调用时捕获的。


此实践并不是关于强制执行操作的特定顺序。它实际上是关于避免空引用异常。

人们关心空引用异常而不是竞争条件背后的原因需要一些深入的心理学研究。我认为这与修复空引用问题要容易得多有关。一旦这个问题解决了,他们就会在代码上挂一个大大的“任务完成”的横幅,然后打开他们的飞行服。

注意:修复竞态条件可能涉及使用同步标志跟踪处理程序是否应该运行


我从未真正认为这是一个大问题,因为我通常只在可重用组件上的静态方法(等)中防止这种潜在的线程错误,而且我不创建静态事件。

我做错了吗?


所以我来的有点晚了。:)

至于使用null而不是null对象模式来表示没有订阅者的事件,请考虑以下场景。您需要调用一个事件,但是构造对象(EventArgs)并非简单,而且在一般情况下,事件没有订阅者。如果您能够优化代码,在进行构造参数和调用事件的处理工作之前检查是否有订阅者,这将对您大有好处。

With this in mind, a solution is to say "well, zero subscribers is represented by null." Then simply perform the null check before performing your expensive operation. I suppose another way of doing this would have been to have a Count property on the Delegate type, so you'd only perform the expensive operation if myDelegate.Count > 0. Using a Count property is a somewhat nice pattern that solves the original problem of allowing optimization, and it also has the nice property of being able to be invoked without causing a NullReferenceException.

但是请记住,由于委托是引用类型,所以它们可以为空。也许根本没有好办法隐藏这一事实,只支持事件的空对象模式,所以替代方案可能迫使开发人员同时检查空订阅者和零订阅者。那将比目前的情况更糟糕。

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


“为什么显式空检查是‘标准模式’?”

我怀疑其原因可能是空检查的性能更好。

如果你总是在创建事件时订阅一个空委托,会有一些开销:

构造空委托的开销。 构建包含它的委托链的成本。 每次引发事件时调用无意义委托的成本。

(请注意,UI控件通常有大量的事件,其中大多数从未订阅过。必须为每个事件创建一个虚拟订阅者,然后调用它,这可能会严重影响性能。)

我做了一些粗略的性能测试,看看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事件调用线程安全的博客文章,这篇文章是我在提出这个问题的前一天发布的(!)

(我的测试设置可能有缺陷,所以请随意下载源代码并自己检查。任何反馈都非常感谢。)


把你在工地的所有活动都打过去,别插手。Delegate类的设计不可能正确地处理任何其他用法,这一点我将在本文的最后一段解释。

首先,当您的事件处理程序必须已经就是否/如何响应通知做出同步决策时,尝试拦截事件通知是没有意义的。

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

处理程序不应该收到事件发生通知的唯一时间是事件实际上没有发生的时候!因此,如果你不想让一个处理程序得到通知,停止生成事件(即禁用控件或任何负责检测并在第一时间使事件存在的东西)。

Honestly, I think the Delegate class is unsalvageable. The merger/transition to a MulticastDelegate was a huge mistake, because it effectively changed the (useful) definition of an event from something that happens at a single instant in time, to something that happens over a timespan. Such a change requires a synchronization mechanism that can logically collapse it back into a single instant, but the MulticastDelegate lacks any such mechanism. Synchronization should encompass the entire timespan or instant the event takes place, so that once an application makes the synchronized decision to begin handling an event, it finishes handling it completely (transactionally). With the black box that is the MulticastDelegate/Delegate hybrid class, this is near impossible, so adhere to using a single-subscriber and/or implement your own kind of MulticastDelegate that has a synchronization handle that can be taken out while the handler chain is being used/modified. I'm recommending this, because the alternative would be to implement synchronization/transactional-integrity redundantly in all your handlers, which would be ridiculously/unnecessarily complex.


根据Jeffrey Richter在《CLR via 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);

因为它强制引用副本。 有关更多信息,请参阅本书中的事件部分。


我真的很喜欢这篇文章——不是!即使我需要它与称为事件的c#功能一起工作!

为什么不在编译器中修复这个问题呢?我知道有MS的人读了这些帖子,所以请不要激怒它!

1 - Null问题)为什么不让事件是。empty而不是Null在第一个地方?有多少行代码将被保存为空检查或必须在声明中插入= 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 -竞态条件问题)我读了Eric的博客文章,我同意H(处理程序)应该处理当它解引用自己,但不能使事件不可变/线程安全?IE,设置一个锁标志在它的创建,这样每当它被调用,它锁定所有订阅和取消订阅,而它的执行?

结论,

现代语言不应该为我们解决这些问题吗?


请看这里:http://www.danielfortunov.com/software/%24daniel_fortunovs_adventures_in_software_development/2009/04/23/net_event_invocation_thread_safety 这是正确的解决方案,应该经常使用,而不是所有其他变通方法。

通过使用不做任何事情的匿名方法初始化内部调用列表,可以确保它始终至少有一个成员。因为没有外部方可以引用匿名方法,也没有外部方可以删除该方法,因此委托永远不会为空。” -编程。net组件,第二版,由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);  
}  

我一直在使用这种设计模式来确保事件处理程序在取消订阅后不会执行。到目前为止,它工作得很好,尽管我还没有尝试过任何性能分析。

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,当你试图在Activity被发送到后台后更新视图时,Android似乎不喜欢它。


我不认为这个问题仅限于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();
    }

Test

我用以下方式测试了它。我有一个线程,它创建和破坏对象,像这样:

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

在一个酒吧(监听器对象)构造函数中,我订阅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#中实现线程安全事件是可能的。

你怎么看?


在c# 6及以上版本中,可以使用new ?运算符如下:

TheEvent ?。调用(这,EventArgs.Empty);

这里是MSDN文档。