为什么foreach循环在某些情况下不起作用?
我正在使用foreach循环来处理要处理的数据列表(一旦处理就删除了所述数据 – 这是在锁内)。 此方法偶尔会导致ArgumentException。
抓住它会很昂贵,所以我试着追查这个问题,但我无法弄明白。
我已经切换到for循环,问题似乎已经消失了。 有人能解释发生了什么吗? 即使有exception消息,我也不太了解幕后发生了什么。
为什么for循环显然有效? 我是否设置了foreach循环错误或什么?
这几乎是我的循环设置方式:
foreach (string data in new List(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 (running) { // [...] }
编辑:根据要求添加了有关exception的更多信息。
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都给出了我喜欢和投票的答案。 索拉林还花了20分钟写了一些有用的代码。
我可以接受所有这三个并让它分享声誉但唉。
Pavel Minaev是下一个应得的人,所以他获得了荣誉。
感谢帮助好人。 🙂
你的问题是List
的构造函数从IEnumerable
(你调用的)创建一个新列表,它的参数不是线程安全的。 会发生什么事情,虽然这个:
new List(Foo.Requests)
正在执行,另一个线程更改Foo.Requests
。 你必须在通话期间锁定它。
[编辑]
正如Eric指出的那样,另一个问题是List
不保证读者可以安全地阅读,而另一个线程正在改变它。 即并发读者是好的,但并发读者和作者不是。 当您将写入彼此锁定时,您不会将读取锁定在写入上。
你的锁定方案已经破裂。 您需要在整个循环期间锁定Foo.Requests()
,而不仅仅是在删除项目时。 否则,该项目可能会在“处理数据”操作过程中变为无效,并且枚举可能会在从一个项目移动到另一个项目之间发生变化。 这假设您不需要在此间隔期间插入集合。 如果是这种情况,您确实需要重新考虑使用适当的生产者/消费者队列。
看到你的例外后; 它看起来我正在构建浅拷贝时改变Foo.Requests。 将其更改为以下内容:
List requests; lock (Foo.Requests) { requests = new List (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); /// /// Called to start processing /// public void Start() { _stopProcessing.Reset(); Thread thread = new Thread(ProcessRequests); thread.Start(); } /// /// Called to request a graceful shutdown of the processing thread /// public void Stop() { _stopProcessing.Set(); // Optionally wait for thread to terminate here _processingStopped.WaitOne(); } /// /// This method does the actual work /// 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(); } /// /// This method will be called when a new requests gets added to the queue /// private void OnRequestAdded() { _processingNeeded.Set(); } } static class Foo { public delegate void RequestAddedHandler(); public static event RequestAddedHandler RequestAdded; static Foo() { Requests = new Queue(); } public static Queue 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检查周围没有锁定。
说实话,我建议重构一下。 您正在迭代对象,同时也在迭代它。 在处理完所有项目之前,您的循环实际上可能会退出。
三件事:
– 我不会将它们锁定在for(每个)语句中,但不在其中。
– 我不会锁定实际的集合,而是一个本地静态对象
– 您无法修改您要枚举的列表/集合
有关更多信息,请查看
http://msdn.microsoft.com/en-us/library/c5kehkcz(VS.80).aspx
lock (lockObject) { foreach (string data in new List(Foo.Requests)) Foo.Requests.Remove(data); }
问题在于表达方式
new List(Foo.Requests)
在你的foreach里面,因为它没有锁定。 我假设当.NET将您的请求集合复制到新列表中时,该列表会被另一个线程修改
foreach (string data in new List(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 todos = null; List empty = new List (); lock(Foo.Requests) { todos = Foo.Requests; Foo.Requests = empty; } //now process local list todos
即便如此,所有共享Foo.Requests的人都必须参与锁定方案。
在迭代列表时,您正尝试从列表中删除对象。 (好的,从技术上讲,你不是这样做的,但那是你想要达到的目标)。
以下是您正确执行此操作的方法:在迭代时,构建另一个要删除的条目列表。 只需构造另一个(临时)列表,将要从原始列表中删除的所有条目放入临时列表中。
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”方法。
我知道这不是你要求的,但仅仅是为了我自己的理智,以下是代表你的代码的意图:
private object _locker = new object(); // ... lock (_locker) { Foo.Requests.Clear(); }