System.AccessViolationException

142 views
Skip to first unread message

djc

unread,
Jul 5, 2006, 6:40:50 PM7/5/06
to
I get this *intermittently* on a utility I am working on. I don't know whats
going on but here are a few points about it:
- using VS 2005, running on xp sp2
- program uses multiple threadpool threads
- only happens when built with the 'release' configuration (no debug flag,
compiler optimizations in effect) - no problems in debug config

here is the error:
Unhandled Exception: System.AccessViolationException: Attempted to read or
write protected memory. This is often an indi
cation that other memory is corrupt.

these two are from the Event Viewer:
Faulting application apreset.exe, version 1.0.0.0, faulting module
msvcr80.dll, version 8.0.50727.42, fault address 0x00014e08.

Faulting application apreset.exe, version 1.0.0.0, stamp 44abbc3b, faulting
module msvcr80.dll, version 8.0.50727.42, stamp 4333a455, debug? 0, fault
address 0x00014e08.

Unfortunately I have to get to some other things and won't be able to work
on this for a few days (I'm stumped anyway). I did not realize any problems
until I built this for release. Always ran fine in debug. The only recent
change was to add a global variable using 'volatile' in the primary thread
because the variable is set only once at the begining of the program (before
any other threads are even launched) but read by several other threads
during their execution. I have not rolled back that change to see if that is
the issue but I will try that when I can get back to this.

upon googling this error I saw many reported issues in .net 2.0 but none
really fitting my scenario. I was hoping some of you kind experts here may
shed some light on this for me? This was my first multithreaded program and
I was real psyched to see it working correctly (in debug) so I'm kind of
hoping it is not a coding issue but maybe something that will be fixed with
the upcoming service pack?

any input is appreciated.


Nicholas Paldino [.NET/C# MVP]

unread,
Jul 5, 2006, 7:00:00 PM7/5/06
to
djc,

I don't know if it is the volatile keyword, but I can tell you that if
you know that it is only going to be read after it is written to once, then
don't use it.

Also, is there any other stack information for the exception that is
thrown?

If worse comes to worse, you can always distribute the debug version.

Hope this helps.


--
- Nicholas Paldino [.NET/C# MVP]
- m...@spam.guard.caspershouse.com

"djc" <no...@nowhere.com> wrote in message
news:ueVCSQIo...@TK2MSFTNGP04.phx.gbl...

Barry Kelly

unread,
Jul 5, 2006, 7:08:15 PM7/5/06
to
"djc" <no...@nowhere.com> wrote:

> I get this *intermittently* on a utility I am working on. I don't know whats
> going on but here are a few points about it:
> - using VS 2005, running on xp sp2
> - program uses multiple threadpool threads
> - only happens when built with the 'release' configuration (no debug flag,
> compiler optimizations in effect) - no problems in debug config

Are you using C++/CLI? Do you have unmanaged components? Are you using
third-party DLLs? Are you using unsafe code? Are you using P/Invoke? Are
you using third-party .NET components that you suspect are using
unmanaged code, unmanaged DLLs or unsafe code?

-- Barry

--
http://barrkel.blogspot.com/

djc

unread,
Jul 5, 2006, 7:39:52 PM7/5/06
to
Thanks for the reply Barry,
Its a C# CLI project, no unmanaged components, no third party DLLs, I
honestly don't know what 'unsafe code' is exactly, no P/Invoke, and no to
the third party .net components. I'm really not too experienced with this
(obviously). I *have* programmed in some way shape or form for several years
but this is my first project using c#, and my first project using more than
one thread. I'm actually a network admin but I enjoy creating my own tools.

I'm not real sure where to go from here to debug this. I'm in over my head
it appears. I should have time to devote to this next week. Unfortunately I
have another pressing issue I'm dealing with.

- I'll look up what 'unsafe code' is. I recall options in VS regarding it.
- any general info on what System.AccessViolationException is and what can
cause it?

"Barry Kelly" <barry....@gmail.com> wrote in message
news:4ehoa2loipgqk1pqh...@4ax.com...

Barry Kelly

unread,
Jul 5, 2006, 8:23:43 PM7/5/06
to
"djc" <no...@nowhere.com> wrote:

> Thanks for the reply Barry,
> Its a C# CLI project, no unmanaged components, no third party DLLs, I
> honestly don't know what 'unsafe code' is exactly, no P/Invoke, and no to
> the third party .net components.

OK. You'd have to turn on unsafe code to use it anyway, since it's
disabled by default.

Are you using any classes outside the System namespace and its children?
I.e. in all your "using" declarations at the top of your source files,
do you have any that don't start with "System"?

> I'm really not too experienced with this
> (obviously). I *have* programmed in some way shape or form for several years
> but this is my first project using c#, and my first project using more than
> one thread. I'm actually a network admin but I enjoy creating my own tools.
>
> I'm not real sure where to go from here to debug this. I'm in over my head
> it appears. I should have time to devote to this next week. Unfortunately I
> have another pressing issue I'm dealing with.

I would guess that you might be missing some synchronization (i.e. you
haven't used 'lock' to ensure that only one thread can access a shared
object at a time). That still shouldn't lead to an access violation,
though.

If I were you, and I had the time, I'd try and prune down the
application until I had the smallest app which still caused the error,
and then post it in full somewhere. Ideally, you could get it small
enough to fit inline in a posting here. Then we could have a look and
try and reproduce it, and figure out the problem.

> - I'll look up what 'unsafe code' is. I recall options in VS regarding it.
> - any general info on what System.AccessViolationException is and what can
> cause it?

It's a bad pointer - i.e. a pointer to memory which hasn't been mapped
into the process's address space. Theoretically, it's impossible with
verifiable, managed code. If you managed to do it with verifiable,
managed code (i.e. without unsafe, unmanaged code), then it's possible
you've found a security hole in the CLR or in one of the BCL assemblies.
It's certainly a very serious bug, *especially* if you're not using
unmanaged code.

djc

unread,
Jul 6, 2006, 9:02:54 AM7/6/06
to

"Barry Kelly" <barry....@gmail.com> wrote in message
news:icloa2pv7dfv8c2ut...@4ax.com...

> "djc" <no...@nowhere.com> wrote:
>
>> Thanks for the reply Barry,
>> Its a C# CLI project, no unmanaged components, no third party DLLs, I
>> honestly don't know what 'unsafe code' is exactly, no P/Invoke, and no to
>> the third party .net components.
>
> OK. You'd have to turn on unsafe code to use it anyway, since it's
> disabled by default.
>

yep, still disabled.

> Are you using any classes outside the System namespace and its children?
> I.e. in all your "using" declarations at the top of your source files,
> do you have any that don't start with "System"?

no, but I am using two classes of my own in the main program class and I
have given them my own namespace so I am referencing two different types
like MyNamepace.Example1 var1; and MyNamespace.Example2 var2; but I did not
include MyNamespace in a 'using' directive at the top. Each of the two
classes I am refering to are in there own file. So I have a total of 3
files, 1 for the main program and 1 each for my two custom classes.

>
>> I'm really not too experienced with this
>> (obviously). I *have* programmed in some way shape or form for several
>> years
>> but this is my first project using c#, and my first project using more
>> than
>> one thread. I'm actually a network admin but I enjoy creating my own
>> tools.
>>
>> I'm not real sure where to go from here to debug this. I'm in over my
>> head
>> it appears. I should have time to devote to this next week. Unfortunately
>> I
>> have another pressing issue I'm dealing with.
>
> I would guess that you might be missing some synchronization (i.e. you
> haven't used 'lock' to ensure that only one thread can access a shared
> object at a time). That still shouldn't lead to an access violation,
> though.

That was my first thought so I locked in places where I didn't even think I
needed to to be sure. I guess I still could have missed something though.

>
> If I were you, and I had the time, I'd try and prune down the
> application until I had the smallest app which still caused the error,
> and then post it in full somewhere. Ideally, you could get it small
> enough to fit inline in a posting here. Then we could have a look and
> try and reproduce it, and figure out the problem.
>

how small is small enough to post here? Between all files *including* blank
lines and comments (which I use a lot of) and lines with only { or } it's
about 450 lines. So actual code is probably like 250 to 300 lines.

>> - I'll look up what 'unsafe code' is. I recall options in VS regarding
>> it.
>> - any general info on what System.AccessViolationException is and what
>> can
>> cause it?
>
> It's a bad pointer - i.e. a pointer to memory which hasn't been mapped
> into the process's address space. Theoretically, it's impossible with
> verifiable, managed code. If you managed to do it with verifiable,
> managed code (i.e. without unsafe, unmanaged code), then it's possible
> you've found a security hole in the CLR or in one of the BCL assemblies.
> It's certainly a very serious bug, *especially* if you're not using
> unmanaged code.
>

I'm pretty sure I'm using only managed code. I wouldn't be suprised if my
luck were to run into a CLR bug when trying to learn something like this but
I think the more likely case is a logic error on my part. Usually is ;)

do you think what I described is too much to post here?

Barry Kelly

unread,
Jul 6, 2006, 9:34:41 AM7/6/06
to
"djc" <no...@nowhere.com> wrote:

> how small is small enough to post here? Between all files *including* blank
> lines and comments (which I use a lot of) and lines with only { or } it's
> about 450 lines. So actual code is probably like 250 to 300 lines.

That sounds small enough.

> > It's a bad pointer - i.e. a pointer to memory which hasn't been mapped
> > into the process's address space. Theoretically, it's impossible with
> > verifiable, managed code. If you managed to do it with verifiable,
> > managed code (i.e. without unsafe, unmanaged code), then it's possible
> > you've found a security hole in the CLR or in one of the BCL assemblies.
> > It's certainly a very serious bug, *especially* if you're not using
> > unmanaged code.
>
> I'm pretty sure I'm using only managed code. I wouldn't be suprised if my
> luck were to run into a CLR bug when trying to learn something like this but
> I think the more likely case is a logic error on my part. Usually is ;)
>
> do you think what I described is too much to post here?

No, I'd say you could safely go ahead. I'm interested to see what could
have caused it!

Another detail: are you running on a multi-processor, dual core or
hyperthreaded machine?

djc

unread,
Jul 6, 2006, 11:47:39 AM7/6/06
to

ok. Here it is. No multiprocessor or dual core. I just copy/pasted the code
from VS 2005 into outlook express. All indentation was lost. I'll leave it
as is assuming the best thing to do if your interested would be to paste
into VS which will hopefully put the indentation back in. If not let me know
and I'll repost and try to make it look good by putting all the indentation
back in etc..

quick overview. I have some real crappy Netgear access points setup in a
warehouse (4 of them) that I routinely have to reboot. To make it easier I
wrote a little CLI that rebooted each one, one at a time (synchronously). I
then decided to make it multithreaded so up to all 4 access points could be
rebooted simultaneously with one quick command. Thats what is pasted below.
It works great in debug but has intermittent issues with a release build,
mostly fails but sometimes seems to work. I get the
system.AccessViolationException at times and other times (noticed more
recently) the output is erroneous. I am using my own class that is basically
a datatable (inherits from system.datatable) with one additional method to
display the data to the console screen nicely (with several overloads). The
powerstate of each access point is monitored by sending pings and the
datatable (consoleTable) is updated to reflect this state so I can see whats
going on - verifying a successfull reboot. I'm describing this since this is
coded for my specific situation and you won't be able to actually run the
program without the same setup.

let me know if I should repost this code and try to put the formatting back
in myself for readability. I will gladly do so. This is kind of driving me
nuts. I am not really supposed to be working on this right now but I'm
anxious to find out whats wrong. I'm assuming I have a logic error somewhere
and that may be hard for anyone else to debug without actually being able to
run this in VS. Again, it compiles fine and does run fine in debug config,
just not in release config????

Main Program follows:
----------------------
using System;
using System.Data;
using System.Threading;
namespace APReset

{

class State

{

internal string theAP;

internal EventWaitHandle eventWaitHandle;

public State(string _theAP, EventWaitHandle _eventWaitHandle)

{

this.theAP = _theAP;

this.eventWaitHandle = _eventWaitHandle;

}

}

class Program

{

//http://TheIPAddress/VxWorks/Reboot?

static DCDev.ConsoleTable ct; //used by all threads for nice output to
console

static volatile int top; //used for top position of the cursor for the
ConsoleTables's display() overload

static int Main(string[] args)

{

if (args.Length < 1 || args.Length > 4)

{

string strHelpMessage = "Usage: apreset n [n] [n] [n] \r\n\r\nReplace 'n'
with 1-4 for specific AP";

strHelpMessage += "\r\n1 = building 43 - front\r\n2 = building 43 -
back\r\n3 = building 45 - front";

strHelpMessage += "\r\n4 = building 45 - back";

Console.WriteLine(strHelpMessage);

//remove following ReadLine(), its for debug only

//Console.ReadLine();

return 1;

}

Console.WriteLine(); //to make a space before main program output - cosmetic

top = Console.CursorTop; // get top cursor position for ConsoleTable's
display() overload

// setup the consoleTable for nice output - access here is safe since there
is only 1 thread at this point

// just lock it- having problems - not sure why

ct = new DCDev.ConsoleTable();

lock (ct)

{

DataColumn col = new DataColumn();

col.ColumnName = "AP";

col.DataType = typeof(string);

col.MaxLength = 5;

ct.Columns.Add(col);

ct.PrimaryKey = new DataColumn[] { ct.Columns["AP"] };

col = new DataColumn();

col.ColumnName = "Status";

col.DataType = typeof(string);

col.MaxLength = 40;

ct.Columns.Add(col);

col = new DataColumn();

col.ColumnName = "PowerState";

col.DataType = typeof(string);

col.MaxLength = 12;

ct.Columns.Add(col);

for (int i = 0; i < args.Length; i++)

{

// create row in ct (console table - which is a datatable) for each AP to
work with

DataRow workRow = ct.NewRow();

workRow["AP"] = args[i].Trim();

workRow["Status"] = "Pending";

workRow["PowerState"] = "Up";

ct.Rows.Add(workRow);

}

ct.AcceptChanges();

}

//to make main thread wait until all others are done before finishing

EventWaitHandle[] eventWaitHandles = new EventWaitHandle[args.Length];

// to pass to worker threads

State state;

try

{

for (int i = 0; i < args.Length; i++)

{

// queue up new thread for each AP so all can run simultaneously

eventWaitHandles[i] = new EventWaitHandle(false,
EventResetMode.ManualReset);

state = new State(args[i].Trim(), eventWaitHandles[i]);

ThreadPool.QueueUserWorkItem(new WaitCallback(workerThreadTask), state);

}

// wait for all worker threads to return

WaitHandle.WaitAll(eventWaitHandles);

Console.WriteLine("All done.");

}

catch (Exception ex)

{

Console.WriteLine("An exception occured: {0}", ex.Message.ToString());

return 1;

}

finally

{

//remove following readLine for build - its for debug only

//Console.ReadLine();

}

return 0;

}

static void workerThreadTask(object state)

{

DCDev.APController myAP = new DCDev.APController();

string usr = "RemovedForPost";

string pwd = "RemovedForPost";

string baseURL = "http://10.10.3.";

string baseIP = "10.10.3.";

State stateInfo = (State)state;

myAP.url = baseURL + stateInfo.theAP + "/VxWorks/Reboot?";

myAP.username = usr;

myAP.password = pwd;

bool firstIteration = true;

bool powerState; //ping result

bool lastPowerState = false; //cache previous result so I only update the
console table if it changed. reduces locks required on ct

//had to set lastPowerState to something here or compiler complained about
using 'unassigned' local var. should not matter what it starts as since it
gets initialized during first iteration of loop

int numOfStateChangesRequired = 2; //default to first ping arriving before
AP actually goes down

UpdateCT(stateInfo.theAP, "Status", "Sending request", "Please wait,
working...");

//if following condition returns true then request was sent and AP responded
OK so we know it was up also

if (myAP.SendRebootRequest())

{

UpdateCT(stateInfo.theAP, "Status", "Request received - rebooting", "Please
wait, working...");

while (numOfStateChangesRequired > 0) //numOfStateChangesRequired defaults
to 2 - assumes first ping is more likely to arrive while AP is still up but
after receiving request to reboot

{

powerState = myAP.IsUp(baseIP + stateInfo.theAP); //returns true if up
(power on)- ping

if (firstIteration)

{

firstIteration = false;

lastPowerState = powerState; //initialize lastPowerState

if (!powerState) //AP went down before first ping arrived; adjust
numOfStateChangesRequired

{

numOfStateChangesRequired = 1;

UpdateCT(stateInfo.theAP, "PowerState", "Down", "Please wait, working...");
//this col defaults to "Up"

}

}

else

{

if (lastPowerState != powerState) //there was a change in power state

{

string colValue;

if (powerState) { colValue = "Up"; } else { colValue = "Down"; }

UpdateCT(stateInfo.theAP, "PowerState", colValue, "Please wait,
working...");

numOfStateChangesRequired--;

}

lastPowerState = powerState;

}

}

//got through all power state changes required so if there was no error,
status is complete

//still need to add check for error - status should change to 'error'

UpdateCT(stateInfo.theAP, "Status", "Complete", "Please wait, working...");

}

else

{

UpdateCT(stateInfo.theAP, "Status", "Error-SendRebootRequest returned
false", "Please wait, working...");

}

stateInfo.eventWaitHandle.Set();

}

static void UpdateCT(string theAP, string colName, string colValue, string
footer)

{

lock (ct)

{

//row should always be found here - shouldn't need to check

DataRow foundRow = ct.Rows.Find(theAP);

foundRow.BeginEdit();

foundRow[colName] = colValue;

foundRow.EndEdit();

ct.AcceptChanges();

ct.Display(0, top, footer); // left, top, - cursor position

}

}

}

}

---------------------------------------------

APController.cs follows:

using System;

using System.Net;

using System.Net.NetworkInformation;

using System.IO;

using System.Text;

namespace DCDev

{

internal class APController

{


private string _url;

private string _usr;

private string _pwd;


public APController(){}


public APController(string url, string usr, string pwd)

{

_url = url;

_usr = usr;

_pwd = pwd;

}


#region PROPERTIES

internal string url

{

get

{

return _url;

}

set

{

_url = value;

}

}

internal string username

{

get

{

return _usr;

}

set

{

_usr = value;

}

}

internal string password

{

get

{

return _pwd;

}

set

{

_pwd = value;

}

}

#endregion

internal bool SendRebootRequest()

{


HttpWebRequest myHTTPRequest = null;

HttpWebResponse myHTTPResponse = null;

StreamReader sr = null;


try

{

myHTTPRequest = (HttpWebRequest)WebRequest.Create(_url);

myHTTPRequest.PreAuthenticate = true;

myHTTPRequest.AllowAutoRedirect = true;

NetworkCredential netCredential = new NetworkCredential();

netCredential.UserName = _usr;

netCredential.Password = _pwd;

myHTTPRequest.Credentials = netCredential;


myHTTPResponse = (HttpWebResponse)myHTTPRequest.GetResponse();

//Console.WriteLine("response status: {0} - from: {1}",
myHTTPResponse.StatusDescription, myHTTPResponse.ResponseUri.Authority);


sr = new StreamReader(myHTTPResponse.GetResponseStream());

string s = sr.ReadToEnd();


//if you ever use this for any APs other than these netgears then the
following 'test' for success needs to change

if (s.IndexOf("Please wait a moment for the AP to reboot...") == -1) return
false;


}

catch (WebException wex)

{

//Console.WriteLine("An exception occured: {0}", wex.Message);

//if (wex.Status == WebExceptionStatus.ProtocolError)

//{

// myHTTPResponse = (HttpWebResponse)wex.Response;

// Console.WriteLine("Protocol Error Code: {0}",
myHTTPResponse.StatusCode.ToString());

//}

return false;

}

finally

{

if (sr != null) sr.Close();

if (myHTTPResponse != null) myHTTPResponse.Close();

}


return true;

}

internal bool IsUp(string ip)

{

// ping - return true if responds, false otherwise


Ping pingSender = new Ping ();

PingOptions options = new PingOptions ();

options.DontFragment = false;

options.Ttl = 128;

// Create buffer of arbitrary data to send in ping packet

string data = "Netgear is gay";

byte[] buffer = Encoding.ASCII.GetBytes (data);

int timeout = 1000; // 1 second

try

{

PingReply reply = pingSender.Send(ip, timeout, buffer, options);

if (reply.Status == IPStatus.Success) { return true; } else { return
false; }

}


catch (Exception ex)

{

//should really do something here to make more robust

return false;

}

}

}

}

-----------------------------------------

ConsoleTable.cs follows:

using System;

using System.Data;

namespace DCDev

{

public class ConsoleTable : DataTable

{

// as it works now this is just a DataTable with my own added method to
display data on console.

// It relies on setting the MaxLenth property of each DataColumn in order to
format the output nicely

public void Display()

{

foreach (DataColumn c in this.Columns)
Console.Write(c.ColumnName.ToString().PadRight(c.MaxLength));

Console.WriteLine();

foreach (DataColumn c in this.Columns)
Console.Write("".PadRight(c.MaxLength, '-'));

Console.WriteLine();


foreach (DataRow r in this.Rows)

{

foreach (DataColumn c in this.Columns)

{

Console.Write(r[c].ToString().PadRight(c.MaxLength));

}

Console.WriteLine();

}

}

public void Display(string footer)

{

foreach (DataColumn c in this.Columns)
Console.Write(c.ColumnName.ToString().PadRight(c.MaxLength));

Console.WriteLine();

foreach (DataColumn c in this.Columns)
Console.Write("".PadRight(c.MaxLength, '-'));

Console.WriteLine();

foreach (DataRow r in this.Rows)

{

foreach (DataColumn c in this.Columns)

{

Console.Write(r[c].ToString().PadRight(c.MaxLength));

}

Console.WriteLine();

}

Console.WriteLine();

Console.WriteLine(footer);

}

public void Display(int x, int y)

{

Console.SetCursorPosition(x, y);

foreach (DataColumn c in this.Columns)
Console.Write(c.ColumnName.ToString().PadRight(c.MaxLength));

Console.WriteLine();

foreach (DataColumn c in this.Columns)
Console.Write("".PadRight(c.MaxLength, '-'));

Console.WriteLine();


foreach (DataRow r in this.Rows)

{

foreach (DataColumn c in this.Columns)

{

Console.Write(r[c].ToString().PadRight(c.MaxLength));

}

Console.WriteLine();

}

}

public void Display(int x, int y, string footer)

{

Console.SetCursorPosition(x, y);

foreach (DataColumn c in this.Columns)
Console.Write(c.ColumnName.ToString().PadRight(c.MaxLength));

Console.WriteLine();

foreach (DataColumn c in this.Columns)
Console.Write("".PadRight(c.MaxLength, '-'));

Console.WriteLine();

foreach (DataRow r in this.Rows)

{

foreach (DataColumn c in this.Columns)

{

Console.Write(r[c].ToString().PadRight(c.MaxLength));

}

Console.WriteLine();

}

Console.WriteLine();

Console.WriteLine(footer);

}

}

}

-------------------------


Barry Kelly

unread,
Jul 6, 2006, 1:59:02 PM7/6/06
to
"djc" <no...@nowhere.com> wrote:

> using System;

Excellent. I have distilled it down to the following:

---8<---


using System;
using System.Net;
using System.Net.NetworkInformation;

class App
{
static void Main()
{
try
{
for (;;)
new Ping().Send(IPAddress.Loopback);
}
catch (Exception ex)
{
if (ex.InnerException != null)
ex = ex.InnerException;
Console.WriteLine("Error: {0} ({1})", ex.Message,
ex.GetType().Name);
}
}
}
--->8---

I've registered a bug at:

http://connect.microsoft.com/VisualStudio/feedback/ViewFeedback.aspx?FeedbackID=157891

To solve your problem, I would wait a few milliseconds between each
iteration of your loops while you poll for a state change. How long does
it normally take for the router or whatever it is to reboot? I'd insert
a Thread.Sleep() for that time period between iterations, to avoid this
race condition.

Adam Clauss

unread,
Jul 6, 2006, 2:27:35 PM7/6/06
to
I confirmed the code as given displays this error after 5-10 seconds (seems
to vary slightly).

I tried a slight variation of the for loop in which I moved the Ping
instantiation outside the for loop and simply called Send on the same
instance each time.
Ping p = new Ping();
for (;;)
{
p.Send(IPAddress.Loopback);
}

This did NOT display the behavior.
Oddly enough, nor did:

for (;;)
{
Ping p = new Ping();
p.Send(IPAddress.Loopback);
}

--
Adam Clauss


djc

unread,
Jul 6, 2006, 3:36:54 PM7/6/06
to
wow. First of all THANK YOU. I'm glad it was not due to improper use of
'lock' or the other synchronization mechanisms I used since I spent a good
deal of time learning about them just for this little tool. Secondly I'm I
little embarrased I didn't narrow it down before posting here. I even had
the try/catch setup there with a code comment to myself that I needed to add
something there in case of error and never did. The fact that it ran ok in
debug really threw me and I immediately thought I was in over my head (which
I was) and posted here.

1) why does it not happen in debug mode? something that the compiler
optimization adds?

2) in your comments to MS feedback you asked why ping was a component. Does
that mean it is 'unmanaged' code?

3) I saw other possible fixes for the scenario here and in the MS feedback
site (same person it looks like). Basically instantiating the ping outside
the for loop first. I think I'll do that. Do you see any issue with that?

Thanks again VERY much for the help.

"Barry Kelly" <barry....@gmail.com> wrote in message

news:fgiqa2dtirbvb7i20...@4ax.com...

djc

unread,
Jul 6, 2006, 3:54:51 PM7/6/06
to
Great, thanks for the information. I will try your change.

strange that your second example does not have the issue??

here is the method that produces the problem in my scenario. I believe it
boils down to your second example except it also instantiates a PingReply
and sends its own arbitrary data in the ping packet. The following method is
called elseware in the program repeatedly within a loop.

internal bool IsUp(string ip)
{


Ping pingSender = new Ping ();
PingOptions options = new PingOptions ();
options.DontFragment = false;
options.Ttl = 128;

// Create buffer of arbitrary data to send in ping packet
string data = "Netgear is gay";
byte[] buffer = Encoding.ASCII.GetBytes (data);
int timeout = 1000; // 1 second

try
{
PingReply reply = pingSender.Send(ip, timeout, buffer, options);
if (reply.Status == IPStatus.Success) { return true; } else {
return false; }
}

catch (Exception ex)
{
Console.WriteLine("Problem is in ping routine!");
Console.WriteLine(ex.Message);
return false;
}
}

"Adam Clauss" <cab...@tamu.edu> wrote in message
news:12aqlf5...@corp.supernews.com...

djc

unread,
Jul 6, 2006, 3:57:30 PM7/6/06
to
whoops! I put your reply in the wrong place. Sorry. This was meant for you:

wow. First of all THANK YOU. I'm glad it was not due to improper use of
'lock' or the other synchronization mechanisms I used since I spent a good
deal of time learning about them just for this little tool. Secondly I'm I
little embarrased I didn't narrow it down before posting here. I even had
the try/catch setup there with a code comment to myself that I needed to add
something there in case of error and never did. The fact that it ran ok in
debug really threw me and I immediately thought I was in over my head (which
I was) and posted here.

1) why does it not happen in debug mode? something that the compiler
optimization adds?

2) in your comments to MS feedback you asked why ping was a component. Does
that mean it is 'unmanaged' code?

3) I saw other possible fixes for the scenario here and in the MS feedback
site (same person it looks like). Basically instantiating the ping outside
the for loop first. I think I'll do that. Do you see any issue with that?

Thanks again VERY much for the help.

"Barry Kelly" <barry....@gmail.com> wrote in message
news:fgiqa2dtirbvb7i20...@4ax.com...

djc

unread,
Jul 6, 2006, 3:58:23 PM7/6/06
to
whoops! this was supposed to be reply to Barry Kelly.

"djc" <no...@nowhere.com> wrote in message

news:%2330oLOT...@TK2MSFTNGP03.phx.gbl...

djc

unread,
Jul 6, 2006, 4:07:41 PM7/6/06
to
...which it was. Man, I really need to get my eyes away from the computer
screen for a while! I thought I put the reply in the wrong place but I
didn't. I'm a tool. No more for me today.

Barry, Adam, Thank you both.

"djc" <no...@nowhere.com> wrote in message

news:%23jdBMaT...@TK2MSFTNGP04.phx.gbl...

Adam Clauss

unread,
Jul 6, 2006, 4:11:46 PM7/6/06
to
"djc" <no...@nowhere.com> wrote in message
news:uu$uNYToG...@TK2MSFTNGP03.phx.gbl...

> Great, thanks for the information. I will try your change.
>
> strange that your second example does not have the issue??

I agree... to me I would think the initial one Barry trimmed out and my last
example would work out exactly the same. The only difference whether the
reference to the Ping object is actually stored in a variable or not...
I kept thinking I was missing something, but I re-ran it several times, and
everytime the intiial example failed while the last two did not. Perhaps
some sort of wierd timing issue? I'm not sure...

I would give Barry's suggestion a try as well - throwing in a Sleep() to
periodically ping rather than constantly ping.

--
Adam Clauss


Barry Kelly

unread,
Jul 6, 2006, 7:17:51 PM7/6/06
to
"djc" <no...@nowhere.com> wrote:

> wow. First of all THANK YOU.

You're welcome. It's an interesting problem. I'm still working on it,
using WinDbg, MSDN docs, .NET Reflector and the SSCLI source code, and
trying to figure out where it all went wrong.

> 1) why does it not happen in debug mode? something that the compiler
> optimization adds?

The code I posted causes problems in debug mode too, on my machine. It
appears to be a race condition with the GC interacting with SafeHandle,
causing a piece of memory used for the reply buffer from the ping to be
disposed of before it can be copied out into the PingReply which
Ping.Send() returns.

> 2) in your comments to MS feedback you asked why ping was a component. Does
> that mean it is 'unmanaged' code?

No. It descends from Component, but doesn't use any Component
functionality. That means it's slightly more awkward to use, and has
some meaningless members like 'Site', 'Container', a public Dispose()
method which doesn't actually dispose the Ping (since it reimplements
IDisposable), and a 'Disposed' event which isn't actually raised by
disposal. To be honest, it looks like a college intern designed the
class - or at least, someone inexperienced in .NET.

> 3) I saw other possible fixes for the scenario here and in the MS feedback
> site (same person it looks like). Basically instantiating the ping outside
> the for loop first. I think I'll do that. Do you see any issue with that?

No. Technically, you should be using a "using" statement to protect the
Ping, like this:

using (Ping ping = new Ping())
{
// work with ping
}

This also stops the exceptions on my machine. I reckon the problem is
down to the Ping instance being disposed before the PingReply has been
fully constructed, although that's a clever trick however the folks at
MS managed to do that - given that the PingReply is new'd up inside an
instance method of Ping.

WinDbg certainly tells me that the buffer the PingReply tries to copy
from has been disposed and closed, and the only place that happens is in
the Ping.InternalDispose() method called by IDispose.Dispose() - which
is never called.

Willy Denoyette [MVP]

unread,
Jul 7, 2006, 6:25:37 AM7/7/06
to

"Barry Kelly" <barry....@gmail.com> wrote in message
news:4gsqa25vi7r2aqu6t...@4ax.com...

Well, it looks like you uncovered a more subtle bug, the Ping class both
derives from Component and implements IDisposable, the function declaration
of Dispose on Ping looks like:
void IDisposable.Dispose(), but Component also implements IDisposable, that
means that if you call Dispose directly without a cast to an IDisposable
interface, you effectively call the Dispose implementation of the base class
(Component) instead of the one that was intended (IDisposable), this results
in a resource (memory) leak because InternalDispose is never called. Note
that this is not a problem when you apply the 'using' statement, but it is
when you rely on the finalizer.

Willy.

Barry Kelly

unread,
Jul 7, 2006, 6:46:54 AM7/7/06
to
"Willy Denoyette [MVP]" <willy.d...@telenet.be> wrote:

> "Barry Kelly" <barry....@gmail.com> wrote in message
> news:4gsqa25vi7r2aqu6t...@4ax.com...
>

> | a public Dispose()
> | method which doesn't actually dispose the Ping (since it reimplements
> | IDisposable)

> Well, it looks like you uncovered a more subtle bug, the Ping class both

> derives from Component and implements IDisposable,

As you can see, I was aware of this already - it's so obvious it was the
first thing I noticed about the class when I started using Ping. It's a
registered bug at MS feedback already.

The real bug, the one that causes the AV, is a lot more subtle than this
trivial one. As I say, I've been investigating, and I'm not sure it's an
error in the Ping class implementation - it looks like some nasty
confluence of GC and SafeHandle conspiring to free a resource too early.
I'm trying to reproduce it now, working with more abstract source based
on Ping.

> Note
> that this is not a problem when you apply the 'using' statement, but it is
> when you rely on the finalizer.

One shouldn't use the finalizer to finalize *other* classes - you can
only finalize yourself. I'm of the opinion that Component should never
have overridden Finalize *at all*. A handle class, ideally descending
from SafeHandle, should be used for finalizing critical resources. One
gains nothing by implementing a "Finalize chain" in addition to the
recommended "IDisposable chain" for logical ownership of resources.

The MSDN documentation on the distinction between when a finalizer is
needed versus the Dispose pattern is very poor, and it's not suprising
that MS programmers get confused.

Willy Denoyette [MVP]

unread,
Jul 7, 2006, 8:26:45 AM7/7/06
to
Barry,

See inline.

Willy.

"Barry Kelly" <barry....@gmail.com> wrote in message

news:09esa21a509fjj556...@4ax.com...


| "Willy Denoyette [MVP]" <willy.d...@telenet.be> wrote:
|
| > "Barry Kelly" <barry....@gmail.com> wrote in message
| > news:4gsqa25vi7r2aqu6t...@4ax.com...
| >
| > | a public Dispose()
| > | method which doesn't actually dispose the Ping (since it reimplements
| > | IDisposable)
|
| > Well, it looks like you uncovered a more subtle bug, the Ping class both
| > derives from Component and implements IDisposable,
|
| As you can see, I was aware of this already - it's so obvious it was the
| first thing I noticed about the class when I started using Ping. It's a
| registered bug at MS feedback already.
|
| The real bug, the one that causes the AV, is a lot more subtle than this
| trivial one. As I say, I've been investigating, and I'm not sure it's an
| error in the Ping class implementation - it looks like some nasty
| confluence of GC and SafeHandle conspiring to free a resource too early.
| I'm trying to reproduce it now, working with more abstract source based
| on Ping.

Sounds reasonable, I wish I had some time to look into this as well.


| > Note
| > that this is not a problem when you apply the 'using' statement, but it
is
| > when you rely on the finalizer.
|
| One shouldn't use the finalizer to finalize *other* classes - you can
| only finalize yourself.

Agreed,the way Ping (which is buggy) is implemented causes the finalizer to
NOT finalize the current Ping intance. The 'correct' implementation should
override the base class Dispose method. But this isn't my point, your sample
relies on the finalizer to dispose of the wrapped unmanaged resources of
Ping, but due to the bug the resources aren't getting disposed of as
intended (Ping.InternalDispose is not called).


I'm of the opinion that Component should never
| have overridden Finalize *at all*. A handle class, ideally descending
| from SafeHandle, should be used for finalizing critical resources.

True, but there is more than handles, there is unmanaged memory blocks which
do not have handles.


One
| gains nothing by implementing a "Finalize chain" in addition to the
| recommended "IDisposable chain" for logical ownership of resources.
|

Agreed.

| The MSDN documentation on the distinction between when a finalizer is
| needed versus the Dispose pattern is very poor, and it's not suprising
| that MS programmers get confused.
|

They should not getting confused, they have internal guidelines which
clearly describe the semantics of both and when they should apply one or the
other, I guess this is one of the bugs that slipped into FCL V2 because of a
lack of time/resources.

Willy.


Barry Kelly

unread,
Jul 7, 2006, 9:10:14 AM7/7/06
to
"Willy Denoyette [MVP]" <willy.d...@telenet.be> wrote:
> "Barry Kelly" <barry....@gmail.com> wrote:
> | The real bug, the one that causes the AV, is a lot more subtle than this
> | trivial one. As I say, I've been investigating, and I'm not sure it's an
> | error in the Ping class implementation - it looks like some nasty
> | confluence of GC and SafeHandle conspiring to free a resource too early.
> | I'm trying to reproduce it now, working with more abstract source based
> | on Ping.
>
> Sounds reasonable, I wish I had some time to look into this as well.

I now have a reproducing case that doesn't use Ping. It's actually quite
interesting, and fairly counter-intuitive. If the behaviour of the GC,
JIT and SafeHandle are all correct (and they are more likely to be
correct than Ping), then I think I've found a new and interesting way to
create bugs! I'm going to write up a blog entry about it this afternoon.

The fact that it involves SafeHandle should be an indication that it
ultimately does only happen in unsafe code, but the mechanism is
something I wasn't aware of. It really is fascinating.

> | I'm of the opinion that Component should never
> | have overridden Finalize *at all*. A handle class, ideally descending
> | from SafeHandle, should be used for finalizing critical resources.
>
> True, but there is more than handles, there is unmanaged memory blocks which
> do not have handles.

Ping actually uses a SafeHandle descendant to free up the unmanaged
memory it uses. The canonical pointer to the start of an unmanaged
memory block (i.e. the one that's going to be passed to the Free method
of the memory allocator) is itself a "handle" to an unmanaged resource.

Finalizers can be used for other things, I won't deny that. For example,
FileStream uses a finalizer to try and flush any pending writes to the
underlying handle.

Reply all
Reply to author
Forward
0 new messages