【问题标题】:Why doesn't a foreach loop work in certain cases?为什么在某些情况下 foreach 循环不起作用?
【发布时间】:2009-07-22 18:44:44
【问题描述】:

我正在使用 foreach 循环遍历要处理的数据列表(处理后删除所述数据 - 这是在锁内)。此方法时不时会导致 ArgumentException。

捕获它会很昂贵,所以我尝试追踪问题,但我无法弄清楚。

我已经切换到一个 for 循环,问题似乎已经消失了。有人可以解释发生了什么吗?即使有异常消息,我也不太明白幕后发生了什么。

为什么 for 循环显然有效?是我错误地设置了 foreach 循环还是什么?

这几乎就是我的循环的设置方式:

foreach (string data in new List<string>(Foo.Requests))
{
    // Process the data.

    lock (Foo.Requests)
    {
        Foo.Requests.Remove(data);
    }
}

和

for (int i = 0; i < Foo.Requests.Count; i++)
{
    string data = Foo.Requests[i];

    // Process the data.

    lock (Foo.Requests)
    {
        Foo.Requests.Remove(data);
    }
}

编辑:for* 循环处于这样的 while 设置中:

while (running)
{
    // [...]
}

编辑:根据要求添加了有关异常的更多信息。

System.ArgumentException: Destination array was not long enough. Check destIndex and length, and the array's lower bounds
  at System.Array.Copy (System.Array sourceArray, Int32 sourceIndex, System.Array destinationArray, Int32 destinationIndex, Int32 length) [0x00000] 
  at System.Collections.Generic.List`1[System.String].CopyTo (System.String[] array, Int32 arrayIndex) [0x00000] 
  at System.Collections.Generic.List`1[System.String].AddCollection (ICollection`1 collection) [0x00000] 
  at System.Collections.Generic.List`1[System.String]..ctor (IEnumerable`1 collection) [0x00000]

编辑:锁定的原因是有另一个线程添加数据。另外,最终会有多个线程在处理数据(所以如果整个设置有误,请指教)。

编辑:很难选择一个好的答案。

我发现 Eric Lippert 的评论值得,但他并没有真正回答(无论如何都对他的评论投了赞成票)。

Pavel Minaev、Joel Coehoorn 和 Thorarin 都给出了我喜欢并投票赞成的答案。 Thorarin 还额外花费了 20 分钟来编写一些有用的代码。

我可以接受所有 3 并让它分裂声誉但是唉。

Pavel Minaev 是下一个应得的,因此他获得了荣誉。

感谢好心人的帮助。 :)

【问题讨论】:

  • 请提供您获得的 ArgumentException 的前几帧(即来自 FCL 本身)。
  • “for”循环正在工作,因为你是幸运。你的线程逻辑完全被破坏了,所以这可能会随机失败。通过更改为 for 循环,您已经巧妙地更改了某些处于竞争状态且不再遇到问题的操作的时间;任何事情都可能导致它回来。如果您想拥有一个在多个线程上读取和修改的集合,那么您需要非常非常小心让您的锁定正确。否则,正如您所经历的那样,它只会随机失败。考虑使用读写锁。
  • @Eric Lippert:为什么这是评论而不是答案?很有帮助。
  • 锁定的原因是什么?上下文是什么? ASP.NET?是否有其他线程访问此数据?
  • @eric 我猜这不是时机。如果没有删除任何项目(即列表中没有项目),for 循环只会遍历列表中的所有项目。如果一个被删除,则列表中的下一个将不会被访问,所以总而言之,只有一半的项目将被删除。而每次删除一个项目时,foreach 循环都会重新开始迭代

标签: c# exception foreach for-loop


【解决方案1】:

您的问题是 List&lt;T&gt; 的构造函数从 IEnumerable (您所称的)创建一个新列表就其参数而言不是线程安全的。发生的情况是:

 new List<string>(Foo.Requests)

正在执行,另一个线程更改Foo.Requests。您必须在通话期间锁定它。

[编辑]

正如 Eric 所指出的,另一个问题List&lt;T&gt; 也不能保证读者在另一个线程正在更改它时安全阅读。 IE。并发读者是可以的,但并发读者和作家不是。而且,当您将写入相互锁定时,您不会将读取与写入锁定。

【讨论】:

    【解决方案2】:

    您的锁定方案已损坏。您需要在整个循环期间锁定Foo.Requests(),而不仅仅是在删除项目时。否则,该项目可能会在您的“处理数据”操作中间变得无效,并且枚举可能会在从一个项目移动到另一个项目之间发生变化。并且假设您也不需要在此时间间隔内插入集合。如果是这种情况,您确实需要重新分解以使用适当的生产者/消费者队列。

    【讨论】:

    • 枚举不会改变,因为他专门制作了一个集合的副本来枚举。 Remove 不会抱怨如果它传递了一个不在集合中的项目(例如,因为另一个线程已经删除了它)。
    • 这可能不是你想要的。处理数据可能需要很长时间,而新数据是异步添加的。话又说回来,这意味着在循环结束后仍有数据需要处理,因为他正在制作 Foo.Requests 的浅表副本。
    • @Pavel:他在复制参考资料。我更多地谈论另一个线程从基本请求集合中删除(并且可能也处理)引用的对象。他最终可能会做双重工作。或者一根线可能会插入中间,他最终可能会遗漏一些东西。
    【解决方案3】:

    看到你的异常后;在我看来, Foo.Requests 在构建浅拷贝时正在更改。把它改成这样:

    List<string> requests;
    
    lock (Foo.Requests)
    {
        requests = new List<string>(Foo.Requests);
    }
    
    foreach (string data in requests)
    {
        // Process the data.
    
        lock (Foo.Requests)
        {
            Foo.Requests.Remove(data);
        }
    }
    

    不是问题,而是……

    话虽如此,我有点怀疑上述内容是否也是您想要的。如果在处理过程中有新的请求进来,当你的 foreach 循环终止时,它们将不会被处理。由于我很无聊,所以我认为您正在努力实现以下目标:

    class RequestProcessingThread
    {
        // Used to signal this thread when there is new work to be done
        private AutoResetEvent _processingNeeded = new AutoResetEvent(true);
    
        // Used for request to terminate processing
        private ManualResetEvent _stopProcessing = new ManualResetEvent(false);
    
        // Signalled when thread has stopped processing
        private AutoResetEvent _processingStopped = new AutoResetEvent(false);
    
        /// <summary>
        /// Called to start processing
        /// </summary>
        public void Start()
        {
            _stopProcessing.Reset();
    
            Thread thread = new Thread(ProcessRequests);
            thread.Start();
        }
    
        /// <summary>
        /// Called to request a graceful shutdown of the processing thread
        /// </summary>
        public void Stop()
        {
            _stopProcessing.Set();
    
            // Optionally wait for thread to terminate here
            _processingStopped.WaitOne();
        }
    
        /// <summary>
        /// This method does the actual work
        /// </summary>
        private void ProcessRequests()
        {
            WaitHandle[] waitHandles = new WaitHandle[] { _processingNeeded, _stopProcessing };
    
            Foo.RequestAdded += OnRequestAdded;
    
            while (true)
            {
                while (Foo.Requests.Count > 0)
                {
                    string request;
                    lock (Foo.Requests)
                    {
                        request = Foo.Requests.Peek();
                    }
    
                    // Process request
                    Debug.WriteLine(request);
    
                    lock (Foo.Requests)
                    {
                        Foo.Requests.Dequeue();
                    }
                }
    
                if (WaitHandle.WaitAny(waitHandles) == 1)
                {
                    // _stopProcessing was signalled, exit the loop
                    break;
                }
            }
    
            Foo.RequestAdded -= ProcessRequests;
    
            _processingStopped.Set();
        }
    
        /// <summary>
        /// This method will be called when a new requests gets added to the queue
        /// </summary>
        private void OnRequestAdded()
        {
            _processingNeeded.Set();
        }
    }
    
    
    static class Foo
    {
        public delegate void RequestAddedHandler();
        public static event RequestAddedHandler RequestAdded;
    
        static Foo()
        {
            Requests = new Queue<string>();
        }
    
        public static Queue<string> Requests
        {
            get;
            private set;
        }
    
        public static void AddRequest(string request)
        {
            lock (Requests)
            {
                Requests.Enqueue(request);
            }
    
            if (RequestAdded != null)
            {
                RequestAdded();
            }
        }
    }
    

    这里还有一些问题,我将留给读者:

    • 应该在每次处理请求后检查 _stopProcessing
    • 如果您有多个线程进行处理,则 Peek() / Dequeue() 方法将不起作用
    • 封装不足:Foo.Requests 是可访问的,但如果您希望处理任何请求,则需要使用 Foo.AddRequest 来添加它们。
    • 如果有多个处理线程:需要在循环内处理队列为空,因为 Count > 0 检查周围没有锁定。

    【讨论】:

    • 有些人主张在整个 foreach 循环中加锁。我被教导锁应该尽可能短(就像这里正在做的一样。这种方法有多可行?它是否适用于多个线程?
    • 它会起作用,但在你的情况下什么是最好的取决于很多事情。处理需要多长时间?如果您将锁放在循环周围,则添加请求的进程将在处理请求时阻塞。我想这不是您想要的,因为在这种情况下,仅使用单线程解决方案会容易得多。
    • 如果您将其简化为伪代码,这大概就是我正在做的事情,但这并不是我正在做的事情。这是我没有想到并且可以借鉴的方法。当然,它不会按原样与多个线程一起使用,但它仍然很有趣,所以感谢分享。 :)
    【解决方案4】:

    老实说,我建议重构它。您正在从对象中删除项目,同时还对其进行迭代。您的循环实际上可能在您处理完所有项目之前退出。

    【讨论】:

    • 如果他要从他正在迭代的源中删除项目,那么他每次都会查看一个非常大的异常。这里不是这样。
    • 然而,问题是关于 foreach 的。但我同意你的重构建议:)
    【解决方案5】:

    三件事:
    - 我不会将它们锁定在 for(each) 语句中,而是在它之外。
    - 我不会锁定实际集合,而是锁定本地静态对象
    - 您不能修改正在枚举的列表/集合

    更多信息请查看:
    http://msdn.microsoft.com/en-us/library/c5kehkcz(VS.80).aspx

    lock (lockObject) {
       foreach (string data in new List<string>(Foo.Requests))
            Foo.Requests.Remove(data);
    }
    

    【讨论】:

    • 他没有修改正在枚举的集合。他复印了一份。
    • 在第二个示例(for 代码)中,他显然在修改现有集合。不是副本。
    • 是的,但这不会使for 循环失败(尽管如果你不考虑它可能会给出不正确的结果)。他的问题是为什么foreach 失败了。
    • 如果你锁定了 foreach 的内部,列表没有完全锁定。它可能会随着每个循环周期而改变。
    【解决方案6】:

    问题在于表达

    new List<string>(Foo.Requests)
    

    在你的 foreach 里面,因为它没有被锁住。我假设当 .NET 将您的请求集合复制到一个新列表中时,该列表已被另一个线程修改

    【讨论】:

      【解决方案7】:
      foreach (string data in new List<string>(Foo.Requests))
      {
          // Process the data.
          lock (Foo.Requests)
          {
              Foo.Requests.Remove(data);
          }
      }
      

      假设您有两个线程执行此代码。

      在 System.Collections.Generic.List1[System.String]..ctor

      • Thread1 开始处理列表。
      • Thread2 调用 List 构造函数,该构造函数对要创建的数组进行计数。
      • Thread1 更改列表中的项目数。
      • Thread2 的项目数有误。

      您的锁定方案是错误的。在 for 循环示例中甚至是错误的。

      每次访问共享资源时都需要锁定 - 甚至读取或复制它。这并不意味着您需要锁定整个操作。这确实意味着共享此共享资源的每个人都需要参与锁定方案。

      还要考虑防御性复制:

      List<string> todos = null;
      List<string> empty = new List<string>();
      lock(Foo.Requests)
      {
        todos = Foo.Requests;
        Foo.Requests = empty;
      }
      
      //now process local list todos
      

      即便如此,所有共享 Foo.Requests 的人都必须参与锁定方案。

      【讨论】:

        【解决方案8】:

        您在遍历列表时试图从列表中删除对象。 (好吧,从技术上讲,你并没有这样做,但这是你试图实现的目标)。

        正确的做法如下:在迭代时,构造另一个要删除的条目列表。只需构建另一个(临时)列表,将要从原始列表中删除的所有条目放入临时列表中。

        List entries_to_remove = new List(...);
        
        foreach( entry in original_list ) {
           if( entry.someCondition() == true ) { 
              entries_to_remove.add( entry );
           }
        }
        
        // Then when done iterating do: 
        original_list.removeAll( entries_to_remove );
        

        使用 List 类的“removeAll”方法。

        【讨论】:

          【解决方案9】:

          我知道这不是您要求的,但只是为了我自己的理智,以下是否代表您的代码的意图:

          private object _locker = new object();
          
          // ...
          
          lock (_locker) {
              Foo.Requests.Clear();
          }
          

          【讨论】:

          • 嗯,不。我不认为是这样。我正在接收数据并将其放入要处理的列表中。即使在处理所述列表时,数据仍会被添加到其中。所以在任何情况下我都不想清除整个列表。
          • 我明白了。我错过了您评论“处理数据”的重要性。
          猜你喜欢
          • 2021-11-01
          • 1970-01-01
          • 1970-01-01
          • 2021-01-18
          • 2021-11-20
          • 2018-11-28
          • 2011-09-04
          • 1970-01-01
          • 1970-01-01
          相关资源
          最近更新 更多