2009-06-24 Gonzalo Paniagua Javier <gonzalo@novell.com>

	* WebConnection.cs: 'socket' and 'Data' where being changed by 2
	threads at the same time when there were queued requests and the
	current one was aborted in Connect().
	* HttpWebRequest.cs: 'aborted' is now an int and we use Interlocked to
	access it. Added WebConnection field used when aborting the request.

	Fixes bug #514591 for good.

svn path=/trunk/mcs/; revision=136753

diff --git a/mcs/class/System/System.Net/ChangeLog b/mcs/class/System/System.Net/ChangeLog
index c37b484fc0f5c79849b9e9c73d167b36b55dfc3a..2a6221984c1b6a6dfa835efb66ad9aca8d67ef36 100644 (file)
--- a/mcs/class/System/System.Net/ChangeLog
+++ b/mcs/class/System/System.Net/ChangeLog
@@ -1,3 +1,13 @@
+2009-06-24 Gonzalo Paniagua Javier <gonzalo@novell.com>
+
+       * WebConnection.cs: 'socket' and 'Data' where being changed by 2
+       threads at the same time when there were queued requests and the
+       current one was aborted in Connect().
+       * HttpWebRequest.cs: 'aborted' is now an int and we use Interlocked to
+       access it. Added WebConnection field used when aborting the request.
+
+       Fixes bug #514591 for good.
+
 2009-06-21 Gonzalo Paniagua Javier <gonzalo@novell.com>
 
        * HttpWebRequest.cs: check for an aborted request in Begin* before

diff --git a/mcs/class/System/System.Net/HttpWebRequest.cs b/mcs/class/System/System.Net/HttpWebRequest.cs
index 999290c55cf61745b6f39eb2d1f26343dc898403..74e6f00e6c709331bf2f313d6fd688e1786b571d 100644 (file)
--- a/mcs/class/System/System.Net/HttpWebRequest.cs
+++ b/mcs/class/System/System.Net/HttpWebRequest.cs
@@ -86,7 +86,7 @@ namespace System.Net
                WebAsyncResult asyncWrite;
                WebAsyncResult asyncRead;
                EventHandler abortHandler;
-               bool aborted;
+               int aborted;
                bool gotRequestStream;
                int redirects;
                bool expectContinue;
@@ -98,6 +98,7 @@ namespace System.Net
                object locker = new object ();
                bool is_ntlm_auth;
                bool finished_reading;
+               internal WebConnection WebConnection;
 #if NET_2_0
                DecompressionMethods auto_decomp;
 #endif
@@ -642,7 +643,7 @@ namespace System.Net
                
                public override IAsyncResult BeginGetRequestStream (AsyncCallback callback, object state) 
                {
-                       if (aborted)
+                       if (Aborted)
                                throw new WebException ("The request was canceled.", WebExceptionStatus.RequestCanceled);
 
                        bool send = !(method == "GET" || method == "CONNECT" || method == "HEAD" ||
@@ -734,7 +735,7 @@ namespace System.Net
 
                public override IAsyncResult BeginGetResponse (AsyncCallback callback, object state)
                {
-                       if (aborted)
+                       if (Aborted)
                                throw new WebException ("The request was canceled.", WebExceptionStatus.RequestCanceled);
 
                        if (method == null)
@@ -827,15 +828,14 @@ namespace System.Net
                }
 
                internal bool Aborted {
-                       get { return aborted; }
+                       get { return Interlocked.CompareExchange (ref aborted, 0, 0) == 1; }
                }
 
                public override void Abort ()
                {
-                       if (aborted)
+                       if (Interlocked.CompareExchange (ref aborted, 1, 0) == 1)
                                return;
 
-                       aborted = true;
                        if (haveResponse && finished_reading)
                                return;
 
@@ -1072,7 +1072,7 @@ namespace System.Net
 
                internal void SetWriteStreamError (WebExceptionStatus status, Exception exc)
                {
-                       if (aborted)
+                       if (Aborted)
                                return;
 
                        WebAsyncResult r = asyncWrite;
@@ -1133,7 +1133,7 @@ namespace System.Net
 
                internal void SetWriteStream (WebConnectionStream stream)
                {
-                       if (aborted)
+                       if (Aborted)
                                return;
                        
                        writeStream = stream;
@@ -1168,7 +1168,7 @@ namespace System.Net
 
                internal void SetResponseError (WebExceptionStatus status, Exception e, string where)
                {
-                       if (aborted)
+                       if (Aborted)
                                return;
                        lock (locker) {
                        string msg = String.Format ("Error getting response stream ({0}): {1}", where, status);
@@ -1238,7 +1238,7 @@ namespace System.Net
                internal void SetResponseData (WebConnectionData data)
                {
                        lock (locker) {
-                       if (aborted) {
+                       if (Aborted) {
                                if (data.stream != null)
                                        data.stream.Close ();
                                return;

diff --git a/mcs/class/System/System.Net/WebConnection.cs b/mcs/class/System/System.Net/WebConnection.cs
index 39081f1755177558a5788f00f37f01526943459a..c0b4ac9f42bdbf46b982f30c0dd4ef2e5246ed2d 100644 (file)
--- a/mcs/class/System/System.Net/WebConnection.cs
+++ b/mcs/class/System/System.Net/WebConnection.cs
@@ -45,7 +45,7 @@ namespace System.Net
                Headers,
                Content
        }
-       
+
        class WebConnection
        {
                ServicePoint sPoint;
@@ -58,6 +58,7 @@ namespace System.Net
                byte [] buffer;
                static AsyncCallback readDoneDelegate = new AsyncCallback (ReadDone);
                EventHandler abortHandler;
+               AbortHelper abortHelper;
                ReadState readState;
                internal WebConnectionData Data;
                bool chunkedRead;
@@ -89,8 +90,22 @@ namespace System.Net
                        readState = ReadState.None;
                        Data = new WebConnectionData ();
                        initConn = new WaitCallback (InitConnection);
-                       abortHandler = new EventHandler (Abort);
                        queue = group.Queue;
+                       abortHelper = new AbortHelper ();
+                       abortHelper.Connection = this;
+                       abortHandler = new EventHandler (abortHelper.Abort);
+               }
+
+               class AbortHelper {
+                       public WebConnection Connection;
+
+                       public void Abort (object sender, EventArgs args)
+                       {
+                               WebConnection other = ((HttpWebRequest) sender).WebConnection;
+                               if (other == null)
+                                       other = Connection;
+                               other.Abort (sender, args);
+                       }
                }
 
                bool CanReuse ()
@@ -100,7 +115,7 @@ namespace System.Net
                        return (socket.Poll (0, SelectMode.SelectRead) == false);
                }
                
-               void Connect ()
+               void Connect (HttpWebRequest request)
                {
                        lock (socketLock) {
                                if (socket != null && socket.Connected && status == WebExceptionStatus.Success) {
@@ -126,6 +141,7 @@ namespace System.Net
                                        return;
                                }
 
+                               WebConnectionData data = Data;
                                foreach (IPAddress address in hostEntry.AddressList) {
                                        socket = new Socket (address.AddressFamily, SocketType.Stream, ProtocolType.Tcp);
 
@@ -140,19 +156,29 @@ namespace System.Net
                                        } else {
 #endif
                                                try {
-                                                       if (Data.request == null || Data.request.Aborted)
+                                                       if (request.Aborted)
                                                                return;
                                                        socket.Connect (remote);
                                                        status = WebExceptionStatus.Success;
                                                        break;
+                                               } catch (ThreadAbortException) {
+                                                       // program exiting...
+                                                       Socket s = socket;
+                                                       socket = null;
+                                                       if (s != null)
+                                                               s.Close ();
+                                                       return;
+                                               } catch (ObjectDisposedException exc) {
+                                                       // socket closed from another thread
+                                                       return;
                                                } catch (Exception exc) {
-                                                       // This might be null if the request is aborted
-                                                       if (socket != null) {
-                                                               socket.Close ();
-                                                               socket = null;
+                                                       Socket s = socket;
+                                                       socket = null;
+                                                       if (s != null)
+                                                               s.Close ();
+                                                       if (!request.Aborted)
                                                                status = WebExceptionStatus.ConnectFailure;
-                                                               connect_exception = exc;
-                                                       }
+                                                       connect_exception = exc;
                                                }
 #if NET_2_0
                                        }
@@ -312,7 +338,8 @@ namespace System.Net
                                        nstream = serverStream;
                                }
                        } catch (Exception) {
-                               status = WebExceptionStatus.ConnectFailure;
+                               if (!request.Aborted)
+                                       status = WebExceptionStatus.ConnectFailure;
                                return false;
                        }
 
@@ -574,23 +601,21 @@ namespace System.Net
                void InitConnection (object state)
                {
                        HttpWebRequest request = (HttpWebRequest) state;
+                       request.WebConnection = this;
 
-                       if (status == WebExceptionStatus.RequestCanceled) {
-                               lock (this) {
-                                       busy = false;
-                                       Data = new WebConnectionData ();
-                                       SendNext ();
-                               }
+                       if (request.Aborted)
                                return;
-                       }
 
                        keepAlive = request.KeepAlive;
                        Data = new WebConnectionData ();
                        Data.request = request;
                retry:
-                       Connect ();
+                       Connect (request);
+                       if (request.Aborted)
+                               return;
+
                        if (status != WebExceptionStatus.Success) {
-                               if (status != WebExceptionStatus.RequestCanceled) {
+                               if (!request.Aborted) {
                                        request.SetWriteStreamError (status, connect_exception);
                                        Close (true);
                                }
@@ -598,12 +623,16 @@ namespace System.Net
                        }
                        
                        if (!CreateStream (request)) {
+                               if (request.Aborted)
+                                       return;
+
+                               WebExceptionStatus st = status;
                                if (Data.Challenge != null)
                                        goto retry;
 
                                Exception cnc_exc = connect_exception;
                                connect_exception = null;
-                               request.SetWriteStreamError (status, cnc_exc);
+                               request.SetWriteStreamError (st, cnc_exc);
                                Close (true);
                                return;
                        }
@@ -614,6 +643,9 @@ namespace System.Net
                
                internal EventHandler SendRequest (HttpWebRequest request)
                {
+                       if (request.Aborted)
+                               return null;
+
                        lock (this) {
                                if (!busy) {
                                        busy = true;
@@ -944,20 +976,23 @@ namespace System.Net
                                lock (queue) {
                                        HttpWebRequest req = (HttpWebRequest) sender;
                                        if (Data.request == req) {
-                                               if (!Data.request.FinishedReading)
-                                                       HandleError (WebExceptionStatus.RequestCanceled, null, "Abort");
+                                               if (!req.FinishedReading) {
+                                                       status = WebExceptionStatus.RequestCanceled;
+                                                       Close (false);
+                                                       Data = new WebConnectionData ();
+                                                       if (queue.Count > 0) {
+                                                               Data.request = (HttpWebRequest) queue.Dequeue ();
+                                                               SendRequest (Data.request);
+                                                       }
+                                               }
                                                return;
-                                       } else if (Data.request == null && socket != null) {
-                                               Socket s = socket;
-                                               socket = null;
-                                               s.Close ();
                                        }
 
                                        req.FinishedReading = true;
                                        req.SetResponseError (WebExceptionStatus.RequestCanceled, null, "User aborted");
                                        if (queue.Count > 0 && queue.Peek () == sender) {
                                                queue.Dequeue ();
-                                       } else {
+                                       } else if (queue.Count > 0) {
                                                object [] old = queue.ToArray ();
                                                queue.Clear ();
                                                for (int i = old.Length - 1; i >= 0; i--) {
@@ -965,8 +1000,6 @@ namespace System.Net
                                                                queue.Enqueue (old [i]);
                                                }
                                        }
-                                       if (queue.Count == 0)
-                                               Close (false);
                                }
                        }
                }