C# Multi-Threading Lock Issue

[Guest]

I have a server application. This application has two pieces of shared data.
They are both hashtables. The untroubling hashtable holds the clients data.
The hashtable that is giving me grief represents a list of all of the
clients connected to the server.

Periodically I go through the client list (a.k.a, the irritating hashtable)
using an enumeration to make sure they are still active. The code looks like
this:

IDictionaryEnumerator i = ClientList.GetEnumerator();
while (i.MoveNext())
{
ClientKeepAliveHandler handler = null;
try
{
handler = (ClientKeepAliveHandler)((object[])i.Value)[1];
IAsyncResult ar = handler.BeginInvoke(new
AsyncCallback(this._asyncCallBack), i.Entry);
}
catch (System.Exception ex)
{
_writeError((int)i.Key,ref ex,MethodBase.GetCurrentMethod().Name);
}
}

Now, this iteration continues happliy along and as the asynchronous call
comes back the _asyncCallBack function gets called that looks like this:


DictionaryEntry clientData = (DictionaryEntry)ar.AsyncState;
ClientKeepAliveHandler del = null;

try
{

del = (ClientKeepAliveHandler)((object[])clientData.Value)[1];
del.EndInvoke(ar);
}
catch (Exception ex)
{
ClientKeepAlive -= del;
Exception e = new Exception("Client to be disconnected (most likely due to
inaccessibility).", ex);
_writeError((int)clientData.Key, ref e, MethodBase.GetCurrentMethod().Name,
"SessionId: " + clientData.Key.ToString() + ", Inner Message: " +
e.InnerException.Message);
CloseSession((int)(clientData.Key));
}

Now, if a client cannot be reached (as in un graceful shutdown) the
EndInvoke method raises an exception of "The remote machine doesn't like you"
or something like that. If that happens the code jumps to the error handler,
removes the delegate from the invokation list of the classes KeepAlive event
and sends an email to the developers who may have random bugs in there code.
Then the last line of the error handler closes the session and removes the
clients data. Keep in mind while this code is executing the original code
that started this may still be looping through the hashtable sending off
asynchrounous calls (and is whenever I experience the problem I'm about to
describe). That being said, when the CloseSession function removes an item
from the Hashtable the enumeration bombs with a "oh no the hashtable has
changed" error, as it should.

Now that I have explained all that the question: How should I implement
locking?
The behavior I want is the _asyncCallBack functions should all wait until I
finish looping before even being able to call EndInvoke. So I tried
implementing this in many ways. lock(ClientList), lock(ClientList.SyncRoot),
Hashtable SyncedHash = Hashtable.Synchronized(ClientList) with a
lock(SyncedHash), Monitor.Enter/Exit, lock(SomeRandomObjectForLocking) all
seem to not work. To be clear, I placed the lock statements right before
ClientKeepAlive -= del in the _asyncCallBack function and right before
IDictionaryEnumerator i = ClientList.GetEnumerator() in the original
snipped...help?

 

[Richard Blewett [DevelopMentor]]

Clone the dictionary in the method that checks keep-alive and modify the original list in the callback that says they're dead. You'll need to lock around the clone and modify

Regards

Richard Blewett - DevelopMentor
http://www.dotnetconsult.co.uk/weblog
http://www.dotnetconsult.co.uk

I have a server application. This application has two pieces of shared data.
They are both hashtables. The untroubling hashtable holds the clients data.
The hashtable that is giving me grief represents a list of all of the
clients connected to the server.

Periodically I go through the client list (a.k.a, the irritating hashtable)
using an enumeration to make sure they are still active. The code looks like
this:

IDictionaryEnumerator i = ClientList.GetEnumerator();
while (i.MoveNext())
{
ClientKeepAliveHandler handler = null;
try
{
handler = (ClientKeepAliveHandler)((object[])i.Value)[1];
IAsyncResult ar = handler.BeginInvoke(new
AsyncCallback(this._asyncCallBack), i.Entry);
}
catch (System.Exception ex)
{
_writeError((int)i.Key,ref ex,MethodBase.GetCurrentMethod().Name);
}
}

Now, this iteration continues happliy along and as the asynchronous call
comes back the _asyncCallBack function gets called that looks like this:


DictionaryEntry clientData = (DictionaryEntry)ar.AsyncState;
ClientKeepAliveHandler del = null;

try
{

del = (ClientKeepAliveHandler)((object[])clientData.Value)[1];
del.EndInvoke(ar);
}
catch (Exception ex)
{
ClientKeepAlive -= del;
Exception e = new Exception("Client to be disconnected (most likely due to
inaccessibility).", ex);
_writeError((int)clientData.Key, ref e, MethodBase.GetCurrentMethod().Name,
"SessionId: " + clientData.Key.ToString() + ", Inner Message: " +
e.InnerException.Message);
CloseSession((int)(clientData.Key));
}

Now, if a client cannot be reached (as in un graceful shutdown) the
EndInvoke method raises an exception of "The remote machine doesn't like you"
or something like that. If that happens the code jumps to the error handler,
removes the delegate from the invokation list of the classes KeepAlive event
and sends an email to the developers who may have random bugs in there code.
Then the last line of the error handler closes the session and removes the
clients data. Keep in mind while this code is executing the original code
that started this may still be looping through the hashtable sending off
asynchrounous calls (and is whenever I experience the problem I'm about to
describe). That being said, when the CloseSession function removes an item
from the Hashtable the enumeration bombs with a "oh no the hashtable has
changed" error, as it should.

Now that I have explained all that the question: How should I implement
locking?
The behavior I want is the _asyncCallBack functions should all wait until I
finish looping before even being able to call EndInvoke. So I tried
implementing this in many ways. lock(ClientList), lock(ClientList.SyncRoot),
Hashtable SyncedHash = Hashtable.Synchronized(ClientList) with a
lock(SyncedHash), Monitor.Enter/Exit, lock(SomeRandomObjectForLocking) all
seem to not work. To be clear, I placed the lock statements right before
ClientKeepAlive -= del in the _asyncCallBack function and right before
IDictionaryEnumerator i = ClientList.GetEnumerator() in the original
snipped...help?

[microsoft.public.dotnet.framework]

 

[0to60]

Now that I have explained all that the question: How should I implement
locking?
The behavior I want is the _asyncCallBack functions should all wait until I
finish looping before even being able to call EndInvoke. So I tried
implementing this in many ways. lock(ClientList), lock(ClientList.SyncRoot),
Hashtable SyncedHash = Hashtable.Synchronized(ClientList) with a
lock(SyncedHash), Monitor.Enter/Exit, lock(SomeRandomObjectForLocking) all
seem to not work. To be clear, I placed the lock statements right before
ClientKeepAlive -= del in the _asyncCallBack function and right before
IDictionaryEnumerator i = ClientList.GetEnumerator() in the original
snipped...help?

You are trying to remove one of the delegates in the callback. I'm assuming
this is going off in another thread while the first thread is still
iterating through the map. Are you getting a "the map was modified as you
were iterating through" message? It seems like you would.

The simplest way to do this type of thing is to iterate through a clone of
the map. So, you're iterating through a clone of the map, but removing
elements from the original map. You're not actually modifying the map
you're iterating through, know what I mean?

foreach(ClientKeepAliveHandler handler in ClientList.Values.Clone())
handler.BeginInvoke(...)

 

[Jon Skeet [C# MVP]]

I have a server application. This application has two pieces of shared data.
They are both hashtables. The untroubling hashtable holds the clients data.
The hashtable that is giving me grief represents a list of all of the
clients connected to the server.

Periodically I go through the client list (a.k.a, the irritating hashtable)
using an enumeration to make sure they are still active. The code looks like
this:

IDictionaryEnumerator i = ClientList.GetEnumerator();
while (i.MoveNext())
{
ClientKeepAliveHandler handler = null;
try
{
handler = (ClientKeepAliveHandler)((object[])i.Value)[1];
IAsyncResult ar = handler.BeginInvoke(new
AsyncCallback(this._asyncCallBack), i.Entry);
}
catch (System.Exception ex)
{
_writeError((int)i.Key,ref ex,MethodBase.GetCurrentMethod().Name);
}
}

That seems quite a long way of doing it - why not use foreach?

Anyway, the problem is that you need to lock the table while you're
iterating through it and when you modify it. Either lock its SyncRoot
or on some other reference you can reach from all the places you need
it.

The behavior I want is the _asyncCallBack functions should all wait until I
finish looping before even being able to call EndInvoke.

If you really want that, then you could lock the hashtable throughout
the use and before you call EndInvoke. However, that wouldn't solve
your problem - you could potentially find that the hashtable is being
looped through again before you get as far as closing the session.

The most elegant way which would involve the least locking would be the
way Richard suggested - take a "local" copy of the map (or in your
case, just the values) and then you can iterate through it without
worrying about locking while you're iterating. You would still need to
lock while copying the values out and for any modification, however.