Bug 14673 - NullReferenceException within CookieContainer during HttpWebRequest async usage (EndGetResponse)
Summary: NullReferenceException within CookieContainer during HttpWebRequest async usa...
Status: RESOLVED FIXED
Alias: None
Product: iOS
Classification: Xamarin
Component: Xamarin.iOS.dll ()
Version: 6.4.4
Hardware: Macintosh Mac OS
: --- normal
Target Milestone: Untriaged
Assignee: Martin Baulig
URL:
Depends on:
Blocks:
 
Reported: 2013-09-12 03:26 UTC by Tyson
Modified: 2016-11-11 09:34 UTC (History)
2 users (show)

Tags:
Is this bug a regression?: ---
Last known good build:

Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report on Developer Community or GitHub with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:
Status:
RESOLVED FIXED

Description Tyson 2013-09-12 03:26:59 UTC
I have had a few crash reports in the wild where this occurs. I finally had it occur on me while running on the Simulator. Unfortunatly it was not much more help - I could see exactly where the null ref occurred and why (null entry at index 0 in the 'cookies' list), but I have no idea how it got into that state as all Add methods are protected from null items.

System.NullReferenceException: Object reference not set to an instance of an object
  at System.Net.CookieContainer.CheckExpiration () [0x0002c] in /Developer/MonoTouch/Source/mono/mcs/class/System/System.Net/CookieContainer.cs:205
  at System.Net.CookieContainer.AddCookie (System.Net.Cookie cookie) [0x001a9] in /Developer/MonoTouch/Source/mono/mcs/class/System/System.Net/CookieContainer.cs:169
  at System.Net.CookieContainer.Add (System.Uri uri, System.Net.Cookie cookie) [0x0003b] in /Developer/MonoTouch/Source/mono/mcs/class/System/System.Net/CookieContainer.cs:258
  at System.Net.HttpWebResponse.SetCookie (System.String header) [0x0009f] in /Developer/MonoTouch/Source/mono/mcs/class/System/System.Net/HttpWebResponse.cs:380
  at System.Net.HttpWebResponse.FillCookies () [0x00023] in /Developer/MonoTouch/Source/mono/mcs/class/System/System.Net/HttpWebResponse.cs:353
  at System.Net.HttpWebResponse..ctor (System.Uri uri, System.String method, System.Net.WebConnectionData data, System.Net.CookieContainer container) [0x000af] in /Developer/MonoTouch/Source/mono/mcs/class/System/System.Net/HttpWebResponse.cs:85
  at System.Net.HttpWebRequest.SetResponseData (System.Net.WebConnectionData data) [0x00039] in /Developer/MonoTouch/Source/mono/mcs/class/System/System.Net/HttpWebRequest.cs:1389

The 'cookies' list contains a null reference at index 0, causing the null ref exception in the for loop.

		// Only needs to be called from AddCookie (Cookie) and GetCookies (Uri)
		void CheckExpiration ()
		{
			if (cookies == null)
				return;

			for (int i = cookies.Count - 1; i >= 0; i--) {
				Cookie cookie = cookies [i];
				if (cookie.Expired)
					cookies.List.RemoveAt (i);
			}
		}
Comment 1 Tyson 2013-10-21 07:08:33 UTC
This has been occurring relatively regularly for me with the debugger attached (not so much without). 

I had a look through the different threads at the time of the crash and it is definitely a race condition that occurs with multiple threads sharing the same CookieContainer. This can occur when multiple HttpWebRequest are created with a shared CookieContainer, and executed asynchronously in parallel.

I havent tried to build a reproducing sample, but I'm pretty sure a very simple app that kicks off 10 - 20 async web requests to the same server, using a single CookieContainer instance will reproduce it. The trick being the target server being requested must assign a session id cookie - at least 2 of the requests will arrive with no existing id, each will get a new session id, the 1st to return will set the cookie, the 2nd will remove the old cookie and update it with the new session id, and then a 3rd will be in the same process when the old cookie gets removed, leaving a null entry that it will hit and throw a NullRef on.

Please investigate this ASAP, I have been waiting a while. It happens less on Release builds (due to the race condition nature) but still does happen. And due to where the exception is thrown in the async callback, I have no opportunity from my code to do anything about it - the request simply never returns or fires any error callbacks.
Comment 2 Tyson 2013-10-23 19:30:47 UTC
Ugh, so it seems the existing code is correct (CookieContainer is not thread safe) - which really makes it difficult to implement shared session async HttpWebRequests as each HttpWebResponse manipulates it's cookie container within internal code on its own IO completion port thread.

http://stackoverflow.com/questions/9911744/is-it-safe-to-use-the-same-cookiecontainer-across-multiple-httpwebrequests
Comment 3 Martin Baulig 2016-11-11 09:34:29 UTC
Closing ancient bugs, please reopen if you're still having this problem.