C#事件和线程安全




multithreading events (10)

UPDATE

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

SomeEvent?.Invoke(this, e);

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

在检查它为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访问器中进行适当同步的自定义事件实现。 此外,如果在事件触发时进行锁定,则存在可能的死锁问题。

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

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

更新:如果不明确,我确实掌握了建议的意图 - 在所有情况下避免空引用异常。 我的意思是,只有当另一个线程从事件中退出时,才会发生此特定的空引用异常,并且唯一的原因是确保不会通过该事件接收到其他调用,这显然不是通过此技术实现的。 你会隐瞒一个竞争条件 - 最好揭示它! 该空例外有助于检测对您的组件的滥用情况。 如果你希望你的组件免受滥用,你可以按照WPF的例子 - 在你的构造函数中存储线程ID,然后在另一个线程试图直接与你的组件交互时抛出一个异常。 或者实现一个真正的线程安全组件(不是一件容易的事)。

所以我争辩说,仅仅做这个复制/检查习惯用法就是货物崇拜编程,给你的代码添加混乱和噪音。 要实际防止其他线程需要更多的工作。

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

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

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

因此,我的问题的剩余部分是, 为什么显式空检查“标准模式”? 另一种方法是分配空的委托,只需要将= delegate {}添加到事件声明中,这样就可以避免每一个事件发生的地方都有那么多的臭味仪式。 很容易确保空委托实例化便宜。 还是我仍然想念一些东西?

肯定肯定是这样的(正如Jon Skeet所说),这只是.NET 1.x的建议,并没有消失,因为它应该在2005年完成。


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

我怀疑这可能是因为空检查更高效。

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

  • 构建空白委托的成本。
  • 构建委托链来包含它的代价。
  • 每次提出事件时调用无意义委托的代价。

(请注意,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事件调用线程安全性的博客文章。

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


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

测试

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

你怎么看?



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

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

使用空白的委托可以解决问题,但也会导致对该事件的每次调用都会导致性能下降,并可能会影响GC。

如果消费者为了实现此目的而取消订阅,您是正确的,但如果他们超过了临时副本,那么请考虑已在传输中的消息。

如果您不使用临时变量,并且不使用空的委托,并且有人退订,那么您会得到一个空引用异常,这是致命的,所以我认为这个代价是值得的。


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

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


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

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

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


我真的很喜欢这个阅读 - 不是! 尽管我需要它与称为事件的C#功能一起工作!

为什么不在编译器中解决这个问题? 我知道有MS读者阅读这些帖子,所以请不要激怒这个!

1 - 空问题 )为什么不把事件放在第一位,而是空的而不是空? 多少行代码将被保存为空检查或必须将a = delegate {}粘贴到声明上? 让编译器处理Empty情况,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,在其创建时设置一个锁标志,以便每当它被调用时,它在执行时锁定所有的订阅和取消订阅?

结论

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


根据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);

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


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

如果没有volatile修饰符,可能会导致本地副本过期,但仅此而已。 它不会导致NullReferenceException

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

TheEvent(this, EventArgs.Empty);

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

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





events