【问题标题】:AutoResetEvent not blocking properlyAutoResetEvent 没有正确阻塞
【发布时间】:2010-08-26 09:09:20
【问题描述】:

我有一个线程,它创建可变数量的工作线程并在它们之间分配任务。这可以通过向线程传递一个 TaskQueue 对象来解决,您将在下面看到其实现。

这些工作线程只是简单地遍历它们所获得的 TaskQueue 对象,执行每个任务。

private class TaskQueue : IEnumerable<Task>
{
    public int Count
    {
        get
        {
            lock(this.tasks)
            {
                return this.tasks.Count;
            }
        }
    }

    private readonly Queue<Task> tasks = new Queue<Task>();
    private readonly AutoResetEvent taskWaitHandle = new AutoResetEvent(false);

    private bool isFinishing = false;
    private bool isFinished = false;

    public void Enqueue(Task task)
    {
        Log.Trace("Entering Enqueue, lock...");
        lock(this.tasks)
        {
            Log.Trace("Adding task, current count = {0}...", Count);
            this.tasks.Enqueue(task);

            if (Count == 1)
            {
                Log.Trace("Count = 1, so setting the wait handle...");
                this.taskWaitHandle.Set();
            }
        }
        Log.Trace("Exiting enqueue...");
    }

    public Task Dequeue()
    {
        Log.Trace("Entering Dequeue...");
        if (Count == 0)
        {
            if (this.isFinishing)
            {
                Log.Trace("Finishing (before waiting) - isCompleted set, returning empty task.");
                this.isFinished = true;
                return new Task();
            }

            Log.Trace("Count = 0, lets wait for a task...");
            this.taskWaitHandle.WaitOne();
            Log.Trace("Wait handle let us through, Count = {0}, IsFinishing = {1}, Returned = {2}", Count, this.isFinishing);

            if(this.isFinishing)
            {
                Log.Trace("Finishing - isCompleted set, returning empty task.");
                this.isFinished = true;
                return new Task();
            }
        }

        Log.Trace("Entering task lock...");
        lock(this.tasks)
        {
            Log.Trace("Entered task lock, about to dequeue next item, Count = {0}", Count);
            return this.tasks.Dequeue();
        }
    }

    public void Finish()
    {
        Log.Trace("Setting TaskQueue state to isFinishing = true and setting wait handle...");
        this.isFinishing = true;

        if (Count == 0)
        {
            this.taskWaitHandle.Set();
        }
    }

    public IEnumerator<Task> GetEnumerator()
    {
        while(true)
        {
            Task t = Dequeue();
            if(this.isFinished)
            {
                yield break;
            }

            yield return t;
        }
    }

    IEnumerator IEnumerable.GetEnumerator()
    {
        return GetEnumerator();
    }
}

如您所见,我使用 AutoResetEvent 对象来确保工作线程不会过早退出,即在执行任何任务之前。

简而言之:

  • 主线程通过Enqeueue将任务分配给线程-将任务分配到其TaskQueue
  • 主线程通过调用TaskQueue的Finish()方法通知线程没有更多的任务要执行
  • 工作线程通过调用 TaskQueue 的 Dequeue() 方法检索分配给它的下一个任务

问题是Dequeue()方法经常抛出一个InvalidOperationException,说Queue是空的。如您所见,我添加了一些日志记录,结果表明,AutoResetEvent 不会阻塞 Dequeue(),即使没有调用它的 >Set() 方法。

据我了解,调用 AutoResetEvent.Set() 将允许等待线程继续(之前调用 AutoResetEvent.WaitOne()),然后自动调用 AutoResetEvent.Reset(),阻塞下一个服务员。

那么有什么问题呢?我是不是搞错了什么?我在某处有错误吗? 我现在在上面坐了 3 个小时,但我不知道出了什么问题。 请帮帮我!

非常感谢!

【问题讨论】:

    标签: c# .net multithreading autoresetevent


    【解决方案1】:

    您的出队代码不正确。您检查 Count 是否处于锁定状态,然后飞过裤子的接缝,然后您期望任务会有一些东西。释放锁时不能保留假设:)。您的 Count 检查和 tasks.Dequeue 必须在锁定状态下发生:

    bool TryDequeue(out Tasks task)
    {
      task = null;
      lock (this.tasks) {
        if (0 < tasks.Count) {
          task = tasks.Dequeue();
        }
      }
      if (null == task) {
        Log.Trace ("Queue was empty");
      }
      return null != task;
     }
    

    你的 Enqueue() 代码同样充满了问题。您的入队/出队不能确保进度(即使队列中有项目,您也会有出队线程阻塞等待)。您的Enqueue() 签名错误。总的来说,你的帖子是非常非常糟糕的代码。坦率地说,我认为您在这里咀嚼的次数超过了您可以咬的次数...哦,并且永远不要在锁定状态下登录

    我强烈建议您只使用ConcurrentQueue

    如果您无法访问 .Net 4.0,这里有一个帮助您入门的实现:

    public class ConcurrentQueue<T>:IEnumerable<T>
    {
        volatile bool fFinished = false;
        ManualResetEvent eventAdded = new ManualResetEvent(false);
        private Queue<T> queue = new Queue<T>();
        private object syncRoot = new object();
    
        public void SetFinished()
        {
            lock (syncRoot)
            {
                fFinished = true;
                eventAdded.Set();
            }
        }
    
        public void Enqueue(T t)
        {
            Debug.Assert (false == fFinished);
            lock (syncRoot)
            {
                queue.Enqueue(t);
                eventAdded.Set();
            }
        }
    
        private bool Dequeue(out T t)
        {
            do
            {
                lock (syncRoot)
                {
                    if (0 < queue.Count)
                    {
                        t = queue.Dequeue();
                        return true;
                    }
                    if (false == fFinished)
                    {
                        eventAdded.Reset ();
                    }
                }
                if (false == fFinished)
                {
                    eventAdded.WaitOne();
                }
                else
                {
                    break;
                }
            } while (true);
            t = default(T);
            return false;
        }
    
    
        public IEnumerator<T> GetEnumerator()
        {
            T t;
            while (Dequeue(out t))
            {
                yield return t;
            }
        }
    
        System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator()
        {
            return GetEnumerator();
        }
    }
    

    【讨论】:

    • 我使用的是 .NET 3.5,所以 ConcurrentQueue 是没有问题的。我非常感谢您的意见,但恐怕我不明白您为什么说这是非常糟糕的代码。也许您可以详细说明一下,并添加一些正确的代码?至于您的 TryDequeue() 建议 - 这完全违背了此类的目的。调用 Deqeueue() 方法总是要返回一些东西;如果要完成该过程,则为空的 Task 对象。如果队列中没有任务,它应该等到有一个。请更新您的帖子。
    • “从不锁定登录”。连痕迹都没有?太笼统的刷了一句话。你如何在你的日志框架中调试代码?通过心灵感应?
    • @Dan Tao:我知道很苛刻,而且是故意的。
    • @Steve Twonsend:是的,永远不要在锁定状态下记录或跟踪。由于日志基础设施(文件、数据库等)中完成的循环,您很可能会引入死锁。 SO 本身在codinghorror.com/blog/2008/08/deadlocked.html 的早期发现自己在那个地方
    • @Remus:我猜你是想说BlockingCollection而不是ConcurrrentCollection?微软对ConcurrentCollection的实现不会阻塞。
    【解决方案2】:

    我正在等待更详细的回答,但我只想指出一些非常重要的事情。

    如果您使用的是 .NET 3.5,您可以使用 ConcurrentQueue&lt;T&gt;Rx extensions library 中包含一个向后移植,可用于 .NET 3.5。

    由于您想要阻止行为,您需要将 ConcurrentQueue&lt;T&gt; 包装在 BlockingCollection&lt;T&gt; 中(也可作为 Rx 的一部分)。

    【讨论】:

    • +1,BlockingCollection 是要走的路。我很确定大多数开发人员已经实现了自己的阻塞队列,用于缓冲任务、消息等。我也很确定大多数开发人员(包括我自己)在前几次都弄错了。线程是hard并且可能使用得太频繁了。
    • @Dan Bryant:哈,我什至会更进一步。大多数(如果不是全部)开发人员(我也是其中的)每次都会出错,包括他们最后的尝试,当他们相信他们已经把这一切都弄清楚了,并为自己做得很好而拍拍自己的后背。大多数这些实现都有可能长期处于休眠状态的错误,就像binary search implementation that was considered proven correct for decades before it was revealed to contain a bug
    • @Dan Tao,在这种情况下,让我们希望 MS 做对了 :) 我有一种感觉,随着 4+ 核心处理器和激进的缓存,我们会看到更多这些问题开始出现优化变得越来越普遍。看看象棋这样的工具是否会成为主流会很有趣:msdn.microsoft.com/en-us/devlabs/cc950526.aspx
    • @Dan Bryant:很有可能。实际上,我非常确信,我们的整个并发编程方法将经历某种范式转变,这仅仅是因为这些问题最终变得多么复杂,以及普通人使用我们当前的方法来解决它们是多么困难。我想知道是否存在根本不同的策略。当然,我不知道会是什么。如果我这样做了,我可能会在会议上发表演讲。
    • @Dan:当然。我在我的答案中发布了一个示例,其中 Microsoft 文档对阻塞队列的实现不好。我看过 Joe Duffy msdn.microsoft.com/en-us/magazine/cc163427.aspx#S4 的杂志文章,它有一个阻塞队列,其行为更像 Barrier 类。事实是专家总是弄错。我已经学会不再多疑了。
    【解决方案3】:

    您似乎正在尝试复制一个阻塞队列。 .NET 4.0 BCL 中已经存在一个BlockingCollection。如果 .NET 4.0 不适合您,那么您可以使用此代码。它使用Monitor.WaitMonitor.Pulse 方法而不是AutoResetEvent

    public class BlockingCollection<T>
    {
        private Queue<T> m_Queue = new Queue<T>();
    
        public T Take() // Dequeue
        {
            lock (m_Queue)
            {
                while (m_Queue.Count <= 0)
                {
                    Monitor.Wait(m_Queue);
                }
                return m_Queue.Dequeue();
            }
        }
    
        public void Add(T data) // Enqueue
        {
            lock (m_Queue)
            {
                m_Queue.Enqueue(data);
                Monitor.Pulse(m_Queue);
            }
        }
    }
    

    更新:

    相当肯定如果您希望它对多个生产者和多个消费者是线程安全的(我准备如果有人可以提出反例,则证明是错误的)。当然,您会在互联网上看到示例,但它们都是错误的。其实one such attempt by Microsoft的缺陷在于队列可以得到live-locked

    【讨论】:

    • 非常感谢您的建议,我会查看 Monitor.Wait 和 Monitor.Pulse 并回复您。不过看起来很有希望!还要感谢您建议 BlockingCollection - 我使用的是 .NET 3.5,但它看起来很有趣。
    • @ShdNx:那你就得自己写了。我会坚持使用规范的实现,因为如果从 scatch 编写代码可能会非常棘手。有一个使用Semaphore 的解决方案,但我认为AutoResetEvent 不存在。原因是因为 ARE 不算数,也不能用于从锁内等待(无论如何都是安全的)。就个人而言,我会坚持使用独家的 Monitor 解决方案,因为这是最广为人知的。
    • @ShdNx:没关系,Dan 只是指出作为 Rx 扩展库的一部分存在一个反向移植。如果你不习惯使用我在这里发布的代码,我会同意的。此外,您将获得的不仅仅是琐碎的 AddTake 方法。
    猜你喜欢
    • 2013-05-14
    • 1970-01-01
    • 1970-01-01
    • 1970-01-01
    • 1970-01-01
    • 2019-07-06
    • 2013-08-19
    • 2013-04-19
    • 2017-07-30
    相关资源
    最近更新 更多