Asynchronous call implementation

  • Thread starter Thread starter archana
  • Start date Start date
A

archana

Hi all,

Need some suggestion on implementating locking mechanism.
currently i have multple asynchronous request which have one callback
function. In callback , i am traverssing through dictionary and adding
into some shared dictionary, Callback is shared accross multiple
thread. So to prevent more than one thread trying to add to sharred
dictionary i have enclosed in lock statement.

But I am thinking of removing lock statment from entire callback and
adding lock statment only when looking into shared resource and adding
to shared resource.

My query is will it cause any issue if more than one thread try to
look into dictionary and try to add same value at the same time to
dictionary.

Can anyone please shed some light on this/

thanks in advance.
 
archana said:
Hi all,

Need some suggestion on implementating locking mechanism.
currently i have multple asynchronous request which have one callback
function. In callback , i am traverssing through dictionary and adding
into some shared dictionary, Callback is shared accross multiple
thread. So to prevent more than one thread trying to add to sharred
dictionary i have enclosed in lock statement.

But I am thinking of removing lock statment from entire callback and
adding lock statment only when looking into shared resource and adding
to shared resource.

My query is will it cause any issue if more than one thread try to
look into dictionary and try to add same value at the same time to
dictionary.

Can anyone please shed some light on this/

thanks in advance.
Click to expand...

Short answer: Yes it will cause issues, so keep it as it is.

Longer answer:

I don't know the internal implementation of a dictionary, but I would
*assume* that any lookup won't change the structure, and if this assumption
is correct then if all you were doing were lookups then you wouldn't need to
lock the dictionary and it should be thread-safe. I think this is a
reasonable assumption, but it is still an assumption.

BUT you are also adding. When you add it will change the structure, and this
certaily isn't thread safe. It will cause problems for any other threads
that are accessing the dictionary - including those which are just reading.
So if you just lock for updates but not reads, then your threads which are
reading will eventually break and throw an exception. So you need to keep it
locked for all accesses.

If say the majority of your accesses were reads and updates are infrequent,
then you could look into a locking strategy which allowed parallel reads but
blocked everything on a write. That would require a bit of thought.
Something like: using semaphores (have lots) to control access to the
dictionary, a mutex to only allow one update at a time and the thread which
has the write mutex can then only do the update once it has successfully
been able to grab ALL of the semaphores. This is adding complexity, so
unless you are finding this to be a performance block then I wouldn't
bother.

Hope this helps.
 
I don't know the internal implementation of a dictionary, but I would
*assume* that any lookup won't change the structure, and if this
assumption is correct then if all you were doing were lookups then you
wouldn't need to lock the dictionary and it should be thread-safe. I
think this is a reasonable assumption, but it is still an assumption.
Click to expand...

AFAIK the .NET collections are thread safe for read-only access. But as
soon as there might be one writer, read is no longer safe.

If say the majority of your accesses were reads and updates are
infrequent, then you could look into a locking strategy which allowed
parallel reads but blocked everything on a write. That would require a
bit of thought. Something like: using semaphores (have lots) to control
access to the dictionary, a mutex to only allow one update at a time and
the thread which has the write mutex can then only do the update once it
has successfully been able to grab ALL of the semaphores. This is adding
complexity, so unless you are finding this to be a performance block
then I wouldn't bother.
Click to expand...

? - ReaderWriterLockSlim will do the job.

However, in case of infrequent updates and a not too large dictionary
there is an even more simple solution. Lock only the writers and do the
following steps in the critical section:
- copy the dictionary
- modify the local copy
- replace the global reference to the current dictionary with the local
copy.
From that on the now shared dictionary instance is immutable and
therefore thread safe. And changing the value of a reference is
implicitly thread safe in .NET.

This solution requires no locking for readers at all.


Marcel
 
Need some suggestion on implementating locking mechanism.
currently i have multple asynchronous request which have one callback
function. In callback , i am traverssing through dictionary and adding
into some shared dictionary, Callback is shared accross multiple
thread. So to prevent more than one thread trying to add to sharred
dictionary i have enclosed in lock statement.

But I am thinking of removing lock statment from entire callback and
adding lock statment only when looking into shared resource and adding
to shared resource.

My query is will it cause any issue if more than one thread try to
look into dictionary and try to add same value at the same time to
dictionary.

Can anyone please shed some light on this/
Click to expand...

1)

There are some single operation issues that you need to
handle correctly:
* avoid corruption of data structure when multiple threads
update the data structure
* avoid inconsistent results when one thread read data structure
being updated by another thread
* having updates to data structure being visible to other threads
(and not be in some CPU specific write back cache)

By far the easiest way of handling those are by using lock.

2)

On top of that there may be some multi operation issues that you
also need to handle correct:
* avoid something relevant happening between two operations

This is also best handled by using lock.

3)

For best performance you should lock the minimum code sequence
as possible.

For single operation just the single call.

For multi operation from before the first call to after the
last call.

Arne
 
However, in case of infrequent updates and a not too large dictionary
there is an even more simple solution. Lock only the writers and do the
following steps in the critical section:
- copy the dictionary
- modify the local copy
- replace the global reference to the current dictionary with the local
copy.
From that on the now shared dictionary instance is immutable and
therefore thread safe. And changing the value of a reference is
implicitly thread safe in .NET.

This solution requires no locking for readers at all.
Click to expand...

I don't see how that guarantees that updates are visible to
other treads.

Arne
 
Short answer: Yes it will cause issues
Click to expand...

Not necessarily.
, so keep it as it is.

Longer answer:

I don't know the internal implementation of a dictionary, but I would
*assume* that any lookup won't change the structure, and if this
assumption is correct then if all you were doing were lookups then you
wouldn't need to lock the dictionary and it should be thread-safe. I
think this is a reasonable assumption, but it is still an assumption.

BUT you are also adding. When you add it will change the structure, and
this certaily isn't thread safe. It will cause problems for any other
threads that are accessing the dictionary - including those which are
just reading. So if you just lock for updates but not reads, then your
threads which are reading will eventually break and throw an exception.
So you need to keep it locked for all accesses.
Click to expand...

That is all correct, but it seems as if the alternatives being compared are:
- lock entire callback
- just lock Dictionary read and write (but both)

Arne
 
Marcel Müller said:
AFAIK the .NET collections are thread safe for read-only access. But as
soon as there might be one writer, read is no longer safe.



? - ReaderWriterLockSlim will do the job.
Click to expand...

I wasn't aware of ReaderWriterLockSlim. Thank you.
 
Arne Vajhøj said:
Not necessarily.
Click to expand...

I agree that there is always a chance that you'd get away with it, but I
don't think that trusting to luck is a sensible approach because eventually
it will break - the only question is how long it will go before breaking and
how repeatable it is. So I think my original short answer was correct: "Yes
it will cause issues, so keep it as it is" - i.e. keep locking in place.

That is all correct, but it seems as if the alternatives being compared
are:
- lock entire callback
- just lock Dictionary read and write (but both)
Click to expand...

If it were me then I'd go with Marcel's suggestion of using
ReaderWriterLockSlim.
 
Hi all,

Need some suggestion on implementating locking mechanism.
currently i have multple asynchronous request which have one callback
function. In callback , i am traverssing through dictionary and adding
into some shared dictionary, Callback is shared accross multiple
thread. So to prevent more than one thread trying to add to sharred
dictionary i have enclosed in lock statement.

But I am thinking of removing lock statment from entire callback and
adding lock statment only when looking into shared resource and adding
to shared resource.
Click to expand...
In .NET 4 the System.Collections.Concurrent namespace contains several
thread-safe data structures.
<http://msdn.microsoft.com/en-us/library/system.collections.concurrent.aspx>
http://msdn.microsoft.com/en-us/library/system.collections.concurrent.aspx

regards
A.G.
 
I don't see how that guarantees that updates are visible to
other treads.
Click to expand...

It doesn't, but as long as other threads do not synchronize otherwise to
the changing thread, it makes no difference. It is just as if the other
thread has been some more ahead in time.

And in practice it simply works, since incoherent caches are not that
common.


Marcel
 
It doesn't, but as long as other threads do not synchronize otherwise to
the changing thread, it makes no difference. It is just as if the other
thread has been some more ahead in time.
Click to expand...

The updates must be used by something - otherwise there are no reason
to do them at all.

So something should be missing them.
And in practice it simply works, since incoherent caches are not that
common.
Click to expand...

"not that common" is not a good reason to write code with
concurrency bugs in.

Arne
 
I agree that there is always a chance that you'd get away with it, but I
don't think that trusting to luck is a sensible approach because
eventually it will break - the only question is how long it will go
before breaking and how repeatable it is. So I think my original short
answer was correct: "Yes it will cause issues, so keep it as it is" -
i.e. keep locking in place.
Click to expand...

No.

The problem is:

#But I am thinking of removing lock statment from entire callback and
#adding lock statment only when looking into shared resource and adding
#to shared resource.

So he now has code like:

rettype callback()
{
lock(somelockobject)
{
foobar();
firstsharedobject.dosomething();
barfoo();
secondsharedobject.dosomething();
foobarfoobar();
}
}

and are considering either:

rettype callback()
{
foobar();
lock(somelockobject)
{
firstsharedobject.dosomething();
barfoo();
secondsharedobject.dosomething();
}
foobarfoobar();
}

or:

rettype callback()
{
foobar();
lock(somelockobject)
{
firstsharedobject.dosomething();
}
barfoo();
lock(somelockobject)
{
secondsharedobject.dosomething();
}
foobarfoobar();
}

The first is perfectly threadsafe.

The second is perfectly thread safe in single operation perspective
(and that is unfortunately the traditional meaning of thread safe),
but not thread safe in multi operation context.

So the code may be fine.

Arne
 
In .NET 4 the System.Collections.Concurrent namespace contains several
thread-safe data structures.
<http://msdn.microsoft.com/en-us/library/system.collections.concurrent.aspx>
Click to expand...

Well, these are AFAIR mainly lock-free data structures. This might be
state of the art, but especially the ConcurrentDictionary is not that
trivial to implement that way. And in fact it is slower than an ordinary
dictionary with an ordinary lock in most cases. Unfortunately I did not
bookmark the link to the article where this was tested.
Note that this applies to .NET 4.0. The .NET 4.5 implementations have
been improved.


Marcel
 
Well, these are AFAIR mainly lock-free data structures. This might be
state of the art, but especially the ConcurrentDictionary is not that
trivial to implement that way. And in fact it is slower than an ordinary
dictionary with an ordinary lock in most cases. Unfortunately I did not
bookmark the link to the article where this was tested.
Note that this applies to .NET 4.0. The .NET 4.5 implementations have
been improved.
Click to expand...

And it is still single operation integrity.

Often multi operation integrity is what one needs.

Constructs like:

if(mylist.Count > 0)
{
X o = mylist[0];
mylist.RemoveAt(0);
...
}

and:

mydict[key] = mydict[key] + 1;

Arne
 
On 10.09.2012 03:41, Arne Vajhøj wrote:
[ConcurrentHashMap]
And it is still single operation integrity.

Often multi operation integrity is what one needs.

Constructs like:

if(mylist.Count > 0)
{
X o = mylist[0];
mylist.RemoveAt(0);
Click to expand...

You wont get that happy with RemoveAt in a hash table. ;-)
...
}

and:

mydict[key] = mydict[key] + 1;
Click to expand...

Well, the interface is a bit more sophisticated:

X o;
if (mylist.TryRemove(key, out o))
{ ...
}

mylist.AddOrUpdate(key, k => throw new KeyNotFoundException(), (k,v) =>
v+1);


Marcel
 
Back
Top