C# 事件和线程安全

问题描述

更新

从 C# 6 开始，这个问题的答案是:

SomeEvent?.Invoke(this, e);

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

在检查事件是否为 null 并触发它之前，请务必制作该事件的副本.这将消除线程处理的潜在问题，即事件在检查 null 和触发事件的位置之间的位置变为 null:

//检查/调用前复制事件委托EventHandler copy = TheEvent;如果(复制！= null)复制(这个，EventArgs.Empty)；//调用复制列表上的任何处理程序

更新:我从阅读有关优化的文章中想到，这可能还要求事件成员具有可变性，但 Jon Skeet 在他的回答中指出，CLR 不会优化掉副本.>

但与此同时，为了使这个问题甚至发生，另一个线程必须这样做:

//更好地从事件中删除 - 从现在开始不希望我们的处理程序被调用:otherObject.TheEvent -= OnTheEvent;//很好，现在我们可以确定 OnTheEvent 不会运行...

实际序列可能是这种混合物:

//检查/调用前复制事件委托EventHandler copy = TheEvent;//更好地从事件中删除 - 从现在开始不希望我们的处理程序被调用:otherObject.TheEvent -= OnTheEvent;//很好，现在我们可以确定 OnTheEvent 不会运行...如果(复制！= null)复制(这个，EventArgs.Empty)；//调用复制列表上的任何处理程序

关键是 OnTheEvent 在作者取消订阅后运行，但他们只是专门取消订阅以避免这种情况发生.当然，真正需要的是在 add 和 remove 访问器中具有适当同步的自定义事件实现.此外，如果在触发事件时持有锁，则可能会出现死锁问题.

这就是 Cargo Cult Programming?似乎是这样 - 很多人必须采取这一步来保护他们的代码免受多线程的影响，而实际上在我看来，事件在它们可以用作多线程设计的一部分之前需要比这更加小心.因此，没有特别注意的人可能会忽略这个建议——这对于单线程程序来说根本不是问题，事实上，鉴于大多数在线示例中没有 volatile代码，建议可能根本没有效果.

(并且在成员声明上分配空的 delegate { } 是不是更简单，这样您就不需要在第一个中检查 null地方?)

更新: 如果不清楚，我确实理解了建议的意图 - 在任何情况下都避免空引用异常.我的观点是，这个特殊的空引用异常只能在另一个线程从事件中除名时发生，这样做的唯一原因是确保不会通过该事件接收到进一步的调用，这显然不是通过这种技术实现的.你会隐瞒比赛条件 - 最好公开它！该空异常有助于检测对组件的滥用.如果您希望您的组件免受滥用，您可以按照 WPF 的示例 - 将线程 ID 存储在您的构造函数中，然后在另一个线程尝试直接与您的组件交互时抛出异常.或者实现一个真正线程安全的组件(不是一件容易的事).

所以我认为，仅仅执行这种复制/检查习语就是对货物崇拜编程，给您的代码添加混乱和噪音.要真正防范其他线程需要做更多工作.

响应 Eric Lippert 的博文而更新:

所以我错过了关于事件处理程序的一个主要事情:即使在取消订阅事件之后，事件处理程序也必须在被调用时保持健壮"，因此显然我们只需要关心这种可能性事件委托为 null.是否在任何地方记录了对事件处理程序的要求?

因此:还有其他方法可以解决这个问题；例如，将处理程序初始化为一个永远不会被删除的空操作.但进行空检查是标准模式."

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

这肯定是(正如 Jon Skeet 所建议的)这只是 .NET 1.x 建议并没有像 2005 年那样消失?

解决方案

由于条件的原因，不允许 JIT 执行您在第一部分中谈论的优化.我知道这是不久前作为幽灵提出的，但它是无效的.(我不久前与乔达菲或万斯莫里森核对过；我不记得是哪个了.)

如果没有 volatile 修饰符，获取的本地副本可能会过时，但仅此而已.它不会导致 NullReferenceException.

是的，肯定存在竞争条件 - 但总会有的.假设我们只是将代码更改为:

TheEvent(this, EventArgs.Empty);

现在假设该委托的调用列表有 1000 个条目.在另一个线程取消订阅列表末尾附近的处理程序之前，列表开头的操作完全有可能已经执行.但是，该处理程序仍将被执行，因为它将是一个新列表.(代表是不可变的.)据我所知，这是不可避免的.

使用空委托当然可以避免无效检查，但不能解决竞争条件.它也不能保证您总是看到"变量的最新值.

UPDATE

As of C# 6, the answer to this question is:

SomeEvent?.Invoke(this, e);

---

I frequently hear/read the following advice:

Always make a copy of an event before you check it for null and fire it. This will eliminate a potential problem with threading where the event becomes null at the location right between where you check for null and where you fire the event:

// Copy the event delegate before checking/calling
EventHandler copy = TheEvent;

if (copy != null)
    copy(this, EventArgs.Empty); // Call any handlers on the copied list

Updated: I thought from reading about optimizations that this might also require the event member to be volatile, but Jon Skeet states in his answer that the CLR doesn't optimize away the copy.

But meanwhile, in order for this issue to even occur, another thread must have done something like this:

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

The actual sequence might be this mixture:

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

The point being that OnTheEvent runs after the author has unsubscribed, and yet they just unsubscribed specifically to avoid that happening. Surely what is really needed is a custom event implementation with appropriate synchronisation in the add and remove accessors. And in addition there is the problem of possible deadlocks if a lock is held while an event is fired.

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.

(And isn't it a lot simpler to just assign the empty delegate { } on the member declaration so that you never need to check for null in the first place?)

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

So I contend that merely doing this copy/check idiom is cargo cult programming, adding mess and noise to your code. To actually protect against other threads requires a lot more work.

Update in response to Eric Lippert's blog posts:

So there's a major thing I'd missed about event handlers: "event handlers are required to be robust in the face of being called even after the event has been unsubscribed", and obviously therefore we only need to care about the possibility of the event delegate being null. Is that requirement on event handlers documented anywhere?

And so: "There are other ways to solve this problem; for example, initializing the handler to have an empty action that is never removed. But doing a null check is the standard pattern."

So the one remaining fragment of my question is, why is explicit-null-check the "standard pattern"? The alternative, assigning the empty delegate, requires only = delegate {} to be added to the event declaration, and this eliminates those little piles of stinky ceremony from every place where the event is raised. It would be easy to make sure that the empty delegate is cheap to instantiate. Or am I still missing something?

Surely it must be that (as Jon Skeet suggested) this is just .NET 1.x advice that hasn't died out, as it should have done in 2005?

解决方案

The JIT isn't allowed to perform the optimization you're talking about in the first part, because of the condition. I know this was raised as a spectre a while ago, but it's not valid. (I checked it with either Joe Duffy or Vance Morrison a while ago; I can't remember which.)

Without the volatile modifier it's possible that the local copy taken will be out of date, but that's all. It won't cause a NullReferenceException.

And yes, there's certainly a race condition - but there always will be. Suppose we just change the code to:

TheEvent(this, EventArgs.Empty);

Now suppose that the invocation list for that delegate has 1000 entries. It's perfectly possible that the action at the start of the list will have executed before another thread unsubscribes a handler near the end of the list. However, that handler will still be executed because it'll be a new list. (Delegates are immutable.) As far as I can see this is unavoidable.

Using an empty delegate certainly avoids the nullity check, but doesn't fix the race condition. It also doesn't guarantee that you always "see" the latest value of the variable.

这篇关于C# 事件和线程安全的文章就介绍到这了，希望我们推荐的答案对大家有所帮助，也希望大家多多支持！