[Http]: Fix a potential race condition in the WebConnectionGroup's connection list.
authorMartin Baulig <martin.baulig@xamarin.com>
Wed, 11 Jun 2014 16:50:47 +0000 (18:50 +0200)
committerMartin Baulig <martin.baulig@xamarin.com>
Wed, 11 Jun 2014 17:07:15 +0000 (19:07 +0200)
Remove the "ConnectionState" from the connection list before closing the connection,
this should avoid ServicePoint.SendRequest() ever crashing with a NullReferenceException
due to getting a null return value from WebConnectionGroup.GetConnection().

mcs/class/System/System.Net/WebConnectionGroup.cs

index 0348aeebe4213ab89381860ab52ef1a0a83230b1..5e142154f913704bd614466144dbe5da559aa014 100644 (file)
@@ -70,14 +70,16 @@ namespace System.Net
                        //TODO: abort requests or wait for them to finish
                        lock (sPoint) {
                                closing = true;
-                               foreach (var cnc in connections) {
-                                       if (cnc.Connection == null)
-                                               continue;
-                                       cnc.Connection.Close (false);
-                                       cnc.Connection = null;
+                               var iter = connections.First;
+                               while (iter != null) {
+                                       var cnc = iter.Value.Connection;
+                                       var node = iter;
+                                       iter = iter.Next;
+
+                                       connections.Remove (node);
+                                       cnc.Close (false);
                                        OnConnectionClosed ();
                                }
-                               connections.Clear ();
                        }
                }
 
@@ -120,7 +122,7 @@ namespace System.Net
                ConnectionState FindIdleConnection ()
                {
                        foreach (var cnc in connections) {
-                               if (cnc.Busy  || cnc.Connection == null)
+                               if (cnc.Busy)
                                        continue;
 
                                connections.Remove (cnc);
@@ -140,7 +142,7 @@ namespace System.Net
                                return cnc.Connection;
                        }
 
-                       if (sPoint.ConnectionLimit > connections.Count) {
+                       if (sPoint.ConnectionLimit > connections.Count || connections.Count == 0) {
                                created = true;
                                cnc = new ConnectionState (this);
                                connections.AddFirst (cnc);
@@ -177,14 +179,11 @@ namespace System.Net
                                }
 
                                int count = 0;
-                               for (var node = connections.First; node != null; node = node.Next) {
-                                       var cnc = node.Value;
-
-                                       if (cnc.Connection == null) {
-                                               connections.Remove (node);
-                                               OnConnectionClosed ();
-                                               continue;
-                                       }
+                               var iter = connections.First;
+                               while (iter != null) {
+                                       var cnc = iter.Value;
+                                       var node = iter;
+                                       iter = iter.Next;
 
                                        ++count;
                                        if (cnc.Busy)
@@ -205,7 +204,7 @@ namespace System.Net
                                        if (connectionsToClose == null)
                                                connectionsToClose = new List<WebConnection> ();
                                        connectionsToClose.Add (cnc.Connection);
-                                       cnc.Connection = null;
+                                       connections.Remove (node);
                                }
 
                                recycled = connections.Count == 0;
@@ -224,7 +223,10 @@ namespace System.Net
                }
 
                class ConnectionState : IWebConnectionState {
-                       public WebConnection Connection;
+                       public WebConnection Connection {
+                               get;
+                               private set;
+                       }
 
                        public WebConnectionGroup Group {
                                get;