Bug 5911 - Crash in WebConnectionStream.ReadAll
Summary: Crash in WebConnectionStream.ReadAll
Status: RESOLVED FIXED
Alias: None
Product: Class Libraries
Classification: Mono
Component: System ()
Version: unspecified
Hardware: PC Mac OS
: --- normal
Target Milestone: Untriaged
Assignee: Gonzalo Paniagua Javier
URL:
: 6114 ()
Depends on:
Blocks:
 
Reported: 2012-06-28 13:42 UTC by Matt Crocker
Modified: 2012-08-12 01:11 UTC (History)
5 users (show)

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


Attachments
Uninformed fix (2.08 KB, patch)
2012-06-28 14:26 UTC, Matt Crocker
Details


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 GitHub or Developer Community 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 Matt Crocker 2012-06-28 13:42:30 UTC
We see the following crash about ~100 times a day from users of our MonoDroid app:

Exception: Non-negative number required.
Parameter name: srcOffset
 at System.Buffer.BlockCopy (System.Array src, Int32 srcOffset, System.Array dst, Int32 dstOffset, Int32 count) [0x00000] in <filename unknown>:0
 at System.Net.WebConnectionStream.ReadAll () [0x00000] in <filename unknown>:0
 at System.Net.WebConnectionStream.CheckResponseInBuffer () [0x00000] in <filename unknown>:0
 at System.Net.WebConnection.ReadDone (IAsyncResult result) [0x00000] in <filename unknown>:0

No hardware/OS pattern, and we haven't been able to repro locally. 

Assuming the exception is accurate, WebConnectionStream.ReadBufferOffset must have been set to -1, which can happen via WebConnection.ReadDone at
https://github.com/mono/mono/blob/master/mcs/class/System/System.Net/WebConnection.cs#L456
https://github.com/mono/mono/blob/master/mcs/class/System/System.Net/WebConnection.cs#L495

It looks like a few other people have encountered this problem: http://stackoverflow.com/questions/5301539/mono-2-8-2-system-buffer-blockcopy-error
Comment 1 Marek Safar 2012-06-28 13:46:59 UTC
Could you try with Mono 2.11 it has many fixes in this ares
Comment 2 Matt Crocker 2012-06-28 14:26:59 UTC
Created attachment 2128 [details]
Uninformed fix

Unfortunately we haven't been able to reproduce the problem ourselves, we just hear about it through crash reports from our MonoDroid app.  MonoDroid uses Mono 2.10, so we can't compare with 2.11.

The problem looks like it should happen in 2.11 though, since WebConnectionStream.ReadBufferOffset can definitely be set to -1.

pos = -1 
https://github.com/mono/mono/blob/master/mcs/class/System/System.Net/WebConnection.cs#L456

cnc.readState is ReadState.Content so these two cases don't apply
https://github.com/mono/mono/blob/master/mcs/class/System/System.Net/WebConnection.cs#L458
https://github.com/mono/mono/blob/master/mcs/class/System/System.Net/WebConnection.cs#L472

cnc.chunkedRead is false, so
stream.ReadBufferOffset = -1;
https://github.com/mono/mono/blob/master/mcs/class/System/System.Net/WebConnection.cs#L495 

I don't know enough about WebConnection, but I think the attached patch may be a reasonable fix. There is definitely no reason for pos to be -1 at this point, as that means either stream.ReadBufferOffset = -1 or cnc.chunkStream = new ChunkStream (cnc.buffer, -1, nread, data.Headers);
Comment 3 Chris Hardy [MSFT] 2012-07-13 12:21:58 UTC
This looks to be similar to the following bug - https://bugzilla.xamarin.com/show_bug.cgi?id=6114
Comment 4 Gonzalo Paniagua Javier 2012-07-13 18:54:06 UTC
*** Bug 6114 has been marked as a duplicate of this bug. ***
Comment 5 Gonzalo Paniagua Javier 2012-07-13 19:38:05 UTC
Looking at the code in 'master' there is no way that pos could be -1 at that point. However, there is a difference with mono-2-10. That difference (and a couple other lines) are now in mono-2-10 as of mono-2-10/bfa23f8.
Comment 6 Matt Crocker 2012-07-13 20:15:13 UTC
Thanks Gonzalo. We'll test this with our users as soon as a build is available.

Regarding the state of master though, is it possible for ReadDone to be called with readState == ReadState.Content?  If so there is still a path for pos = -1 to be set on the stream.
Comment 7 Gonzalo Paniagua Javier 2012-08-11 11:18:06 UTC
The first call to ReadDone for any given connection has ReadState.None. Subsequent calls are only scheduled when state != ReadState.Content. What path are you talking about?
Comment 8 Matt Crocker 2012-08-12 01:11:16 UTC
If ReadDone is called with cnc.readState == ReadState.Content then it will get to here https://github.com/mono/mono/blob/master/mcs/class/System/System.Net/WebConnection.cs#L495 with pos = -1.   If that can't happen then cool.

FWIW we've deployed the fix on Android, and it seems to have worked. We haven't seen the exception since.  Thanks for the help!