Hashtable Synchronization to Ensure Thread Safety

[Guest]

I have a question regarding synchronization across multiple threads for a
Hashtable. Currently I have a Threadpool that is creating worker threads
based on requests to read/write to a hashtable. One function of the
Hashtable is to iterate through its keys, which apparently is inherently not
thread-safe. Other functions of the Hashtable include
adding/modifying/deleting.

To solve the synchronization issues I am doing two things:
1. Lock the syncroot of the Hashtable for any "write" operations:
Lock(myHashTable.Syncroot)
{ do some writing/deleting }

2. Wrap the Hashtable in a synchronized Hashtable for any iterations:
Hashtable mySynchedTable = Hashtable.Synchronized(myHashTable);
foreach(key in mySynchedTable)
{ .. do some reading }

My question is: is this enough? Is there anything missing, or should this be
enough to ensure synchronization across multiple threads accessing my
Hashtable.

Thanks!

 

[William Stacey [MVP]]

lock it yourself in all read or write methods using
lock(syncRoot)
{
foreach(...)
update/etc.
}

The problem with sync wrapper is it is locked every access so if you have a
method that does iteration and updates and counts, etc you keep locking it
multiple times. Also, something you need exclusive lock for a set of
operations that need to be in the same lock context to be safe. You could
still use the syncwrapper, but you end up locking twice. It is a bit more
work, but I would set your locks yourself unless your needs are simple.

 

[Guest]

Thanks for the reply. Currently I have it set up to do: lock(myHash.syncRoot)
around any write methods and use a Hashtable.Synchronized wrapper around any
of my iterations like:

Hashtable mySynch = Hashtable.Synchronized(myHash);
foreach (string Key in mySynch.Keys)
{
MyObject tempObject = (MyObject)mySynch[Key];
tempObject.doSomeWork();
}

So from your post, I gather that this Synchronized Wrapper is doing a lock
each iteration of the Hashtable? So I'd be better off doing:
lock(myHash.syncRoot)
{
foreach (string Key in myHash.Keys)
{
MyObject tempObject = (MyObject)myHash[Key];
tempObject.doSomeWork();
}
}

so this would lock only once?

Thanks!

 

[William Stacey [MVP]]

So from your post, I gather that this Synchronized Wrapper is doing a lock

each iteration of the Hashtable?

Not 100% on that. Each method or property access is wrapped in a lock,
hence the name.

So I'd be better off doing:
lock(myHash.syncRoot)
{
foreach (string Key in myHash.Keys)
{
MyObject tempObject = (MyObject)myHash[Key];
tempObject.doSomeWork();
}
}

That is much better then first example. Also, from first example, you would
creating a new wrapper for every access - but can not tell for sure without
the whole class or a small class sample. Normally you end up needing a
lock{} to handle a few atomic operations regarding table and/or other stuff.
So having to handle both the wrapper and manual locks get a bit messy IMO
and have the two lock overhead if using both. For that reason I find easier
to reason about the locking on shared resources if I just do the locks
myself and I think makes your code more understandable as it is explicit
what your doing. But others may disagree. Cheers.

 

[Guest]

Thanks! I'll give it a shot!

So from your post, I gather that this Synchronized Wrapper is doing a lock
each iteration of the Hashtable?

Not 100% on that. Each method or property access is wrapped in a lock,
hence the name.

So I'd be better off doing:
lock(myHash.syncRoot)
{
foreach (string Key in myHash.Keys)
{
MyObject tempObject = (MyObject)myHash[Key];
tempObject.doSomeWork();
}
}

That is much better then first example. Also, from first example, you would
creating a new wrapper for every access - but can not tell for sure without
the whole class or a small class sample. Normally you end up needing a
lock{} to handle a few atomic operations regarding table and/or other stuff.
So having to handle both the wrapper and manual locks get a bit messy IMO
and have the two lock overhead if using both. For that reason I find easier
to reason about the locking on shared resources if I just do the locks
myself and I think makes your code more understandable as it is explicit
what your doing. But others may disagree. Cheers.

 

[Guest]

Actually in .NET 1.1. the synchronized hashtable only actually locks on
removes if I recall correctly.


Some interesting info for people using hashtables:
1. When threading use the synchronized hastable wrapper if you are not
worried about not seeing writes immediately when done from another thread.
2. The synchronized hashtable only locks on deletions internally as it
copies buckets normal to avoid locking
3. Given the above implementation it is slow, but provides good parallelism
and reduced contention
4. Given #1 is ok for you, you only need to lock if you need to enumerate
all of the elements in the hashtable since the enmerator is not thread safe.
5. For even higher performance given a well known dataset and using strings
as keys you might try a different hash algorithm, the MS one is nicely
distributed producing few collisions but is slow compared to other algorithms
that would produce more collisions for large data sets, but if your data set
is well known or small and diverse some more simplistic algorithms may work
for you.

Thanks! I'll give it a shot!

So from your post, I gather that this Synchronized Wrapper is doing a lock
each iteration of the Hashtable?

Not 100% on that. Each method or property access is wrapped in a lock,
hence the name.

So I'd be better off doing:
lock(myHash.syncRoot)
{
foreach (string Key in myHash.Keys)
{
MyObject tempObject = (MyObject)myHash[Key];
tempObject.doSomeWork();
}
}

That is much better then first example. Also, from first example, you would
creating a new wrapper for every access - but can not tell for sure without
the whole class or a small class sample. Normally you end up needing a
lock{} to handle a few atomic operations regarding table and/or other stuff.
So having to handle both the wrapper and manual locks get a bit messy IMO
and have the two lock overhead if using both. For that reason I find easier
to reason about the locking on shared resources if I just do the locks
myself and I think makes your code more understandable as it is explicit
what your doing. But others may disagree. Cheers.