Browse Source

[System.ServiceModel] Fix "collection was modified" exception in tests

We sometimes see the following exception in the ServiceModel tests:

```
System.InvalidOperationException: Collection was modified; enumeration operation may not execute.
  at System.ThrowHelper.ThrowInvalidOperationException (ExceptionResource resource) [0x00000] in /var/lib/jenkins/workspace/test-mono-mainline/label/debian-amd64/external/referencesource/mscorlib/system/throwhelper.cs:99
  at System.Collections.Generic.List`1+Enumerator[T].MoveNextRare () [0x00016] in /var/lib/jenkins/workspace/test-mono-mainline/label/debian-amd64/external/referencesource/mscorlib/system/collections/generic/list.cs:1180
  at System.Collections.Generic.List`1+Enumerator[T].MoveNext () [0x00050] in /var/lib/jenkins/workspace/test-mono-mainline/label/debian-amd64/external/referencesource/mscorlib/system/collections/generic/list.cs:1174
  at System.Linq.Enumerable.FirstOrDefault[TSource] (IEnumerable`1 source, System.Func`2 predicate) [0x00041] in /var/lib/jenkins/workspace/test-mono-mainline/label/debian-amd64/external/referencesource/System.Core/System/Linq/Enumerable.cs:960
  at System.ServiceModel.Channels.Http.HttpListenerManager.TryDequeueRequest (System.ServiceModel.Dispatcher.ChannelDispatcher channel, TimeSpan timeout, System.ServiceModel.Channels.Http.HttpContextInfo& context) [0x00016] in /var/lib/jenkins/workspace/test-mono-mainline/label/debian-amd64/mcs/class/System.ServiceModel/System.ServiceModel.Channels.Http/HttpListenerManager.cs:93
  at System.ServiceModel.Channels.Http.HttpListenerManager.TryDequeueRequest (System.ServiceModel.Dispatcher.ChannelDispatcher channel, TimeSpan timeout, System.ServiceModel.Channels.Http.HttpContextInfo& context) [0x000ab] in /var/lib/jenkins/workspace/test-mono-mainline/label/debian-amd64/mcs/class/System.ServiceModel/System.ServiceModel.Channels.Http/HttpListenerManager.cs:104
  at System.ServiceModel.Channels.Http.HttpReplyChannel.TryReceiveRequest (TimeSpan timeout, System.ServiceModel.Channels.RequestContext& context) [0x00003] in /var/lib/jenkins/workspace/test-mono-mainline/label/debian-amd64/mcs/class/System.ServiceModel/System.ServiceModel.Channels.Http/HttpReplyChannel.cs:154
  at System.ServiceModel.Channels.ReplyChannelBase+<BeginTryReceiveRequest>c__AnonStorey0.<>m__0 (TimeSpan tout, System.ServiceModel.Channels.RequestContext& ctx) [0x00056] in /var/lib/jenkins/workspace/test-mono-mainline/label/debian-amd64/mcs/class/System.ServiceModel/System.ServiceModel.Channels/ReplyChannelBase.cs:129
```

I managed to repro this consistently on the debian-7-x64-1 Jenkins machine by running the test in a loop with the following code for about 15mins:
    while [ $? -eq 0 ]; do make check FIXTURE=System.ServiceModel.Dispatcher; done

The problem happens when RegisterListenerCommon/UnregisterListenerCommon is invoked through HttpChannelListener.OnOpen/OnAbort/OnClose which modifies the 'Entries' collection on another thread.

The fix is to add locking around the collection accesses.
Alexander Köplinger 10 years ago
parent
commit
d33a7df441

+ 20 - 10
mcs/class/System.ServiceModel/System.ServiceModel.Channels.Http/HttpListenerManager.cs

@@ -48,24 +48,29 @@ namespace System.ServiceModel.Channels.Http
 			Entries = new List<HttpChannelListenerEntry> ();
 		}
 
-		public List<HttpChannelListenerEntry> Entries { get; private set; }
+		protected List<HttpChannelListenerEntry> Entries { get; private set; }
+		object entries_lock = new object ();
 
 		public abstract void RegisterListener (ChannelDispatcher channel, HttpTransportBindingElement element, TimeSpan timeout);
 		public abstract void UnregisterListener (ChannelDispatcher channel, TimeSpan timeout);
 
 		protected void RegisterListenerCommon (ChannelDispatcher channel, TimeSpan timeout)
 		{
-			Entries.Add (new HttpChannelListenerEntry (channel, new AutoResetEvent (false)));
+			lock (entries_lock) {
+				Entries.Add (new HttpChannelListenerEntry (channel, new AutoResetEvent (false)));
 
-			Entries.Sort (HttpChannelListenerEntry.CompareEntries);
+				Entries.Sort (HttpChannelListenerEntry.CompareEntries);
+			}
 		}
 
 		protected void UnregisterListenerCommon (ChannelDispatcher channel, TimeSpan timeout)
 		{
-			var entry = Entries.First (e => e.ChannelDispatcher == channel);
-			Entries.Remove (entry);
+			lock (entries_lock) {
+				var entry = Entries.First (e => e.ChannelDispatcher == channel);
+				Entries.Remove (entry);
 
-			entry.WaitHandle.Set (); // make sure to finish pending requests.
+				entry.WaitHandle.Set (); // make sure to finish pending requests.
+			}
 		}
 
 		public void ProcessNewContext (HttpContextInfo ctxi)
@@ -79,9 +84,11 @@ namespace System.ServiceModel.Channels.Http
 
 		HttpChannelListenerEntry SelectChannel (HttpContextInfo ctx)
 		{
-			foreach (var e in Entries)
-				if (e.FilterHttpContext (ctx))
-					return e;
+			lock (entries_lock) {
+				foreach (var e in Entries)
+					if (e.FilterHttpContext (ctx))
+						return e;
+			}
 			return null;
 		}
 
@@ -90,7 +97,10 @@ namespace System.ServiceModel.Channels.Http
 			DateTime start = DateTime.Now;
 
 			context = null;
-			var ce = Entries.FirstOrDefault (e => e.ChannelDispatcher == channel);
+			HttpChannelListenerEntry ce = null;
+			lock (entries_lock) {
+				ce = Entries.FirstOrDefault (e => e.ChannelDispatcher == channel);
+			}
 			if (ce == null)
 				return false;
 			lock (ce.RetrieverLock) {