ReaderWriterLock with Interlocked?

[Guest]

Found this piece of code in .Net SDK 2.0. Why do we need to use Interlocked
although AcquireWriterLock is supposed to serialize access and do the traick?

// C#
ReaderWriterLock rwLock = new ReaderWriterLock();
int counter = 0;

try
{
rwLock.AcquireWriterLock(1000);

try
{
Interlocked.Increment(ref counter);
}
finally
{
rwLock.ReleaseWriterLock();
}
}
catch (ApplicationException)
{
Console.WriteLine("Failed to get a Writer Lock");
}

Thank you for any tips.

 

[Barry Kelly]

Found this piece of code in .Net SDK 2.0. Why do we need to use Interlocked
although AcquireWriterLock is supposed to serialize access and do the traick?

// C#
ReaderWriterLock rwLock = new ReaderWriterLock();
int counter = 0;

try
{
rwLock.AcquireWriterLock(1000);

try
{
Interlocked.Increment(ref counter);
}
finally
{
rwLock.ReleaseWriterLock();
}
}
catch (ApplicationException)
{
Console.WriteLine("Failed to get a Writer Lock");
}

Thank you for any tips.

That depends on whether 'counter' is used without synchronization
elsewhere in the body of code. If it isn't, then the locking is
redundant. (FWIW, it wouldn't be the first crappy piece of code in the
samples with the SDK & docs.)

-- Barry

 

[Michael Nemtsev]

Hello Barry,

int (counter in our case) is atomic, afailk (if alignment wasn't changed).

Shouldn't this to be a reason to avoid Increment at all?!


---
WBR,
Michael Nemtsev [C# MVP] :: blog: http://spaces.live.com/laflour

"The greatest danger for most of us is not that our aim is too high and we
miss it, but that it is too low and we reach it" (c) Michelangelo

BK> Amir The Ruler wrote:
BK>
BK> That depends on whether 'counter' is used without synchronization
BK> elsewhere in the body of code. If it isn't, then the locking is
BK> redundant. (FWIW, it wouldn't be the first crappy piece of code in
BK> the samples with the SDK & docs.)

 

[Guest]

actually that code is copied from MCTS Self-Paced Training Kit (Exam 70-536)
but i verified and found almost the same thing in sdk:

rwl.AcquireWriterLock(timeOut);
try
{
// It is safe for this thread to read or write
// from the shared resource.
resource = rnd.Next(500);
Display("writes resource value " + resource);
Interlocked.Increment(ref writes);

it yet doesn't make sense to use 2 sync mechanisms at once . and those
variables are not used anywhere else without synchronization.

 

[Barry Kelly]

int (counter in our case) is atomic, afailk (if alignment wasn't changed).

Shouldn't this to be a reason to avoid Increment at all?!

Loads and assignments individually are atomic, but increment requires an
atomic combination of "load and increment and assignment" all in the one
"transaction". Thus either you need manual synchronization or
Interlocked.Increment etc.

Incrementing an int32 with a memory operand (e.g. 'inc dword ptr [eax]')
on a single-CPU x86 system, with no dual core and no hyperthreading, is
atomic. Other architectures, more CPUs, etc., almost certainly require
some kind of mutual exclusion or interlocked operation to work.

-- Barry

 

[Henning Krause [MVP - Exchange]]

Hello,

Loads and assignments individually are atomic, but increment requires an
atomic combination of "load and increment and assignment" all in the one
"transaction". Thus either you need manual synchronization or
Interlocked.Increment etc.

However, reading a 64bit value on a 32bit architecture is not atomic and
must be protected as well...

Best regards,
Henning Krause

 

[Barry Kelly]

Hello,


However, reading a 64bit value on a 32bit architecture is not atomic and
must be protected as well...

Of course, we're talking about an int here.

-- Barry

 

[Jon Skeet [C# MVP]]

int (counter in our case) is atomic, afailk (if alignment wasn't changed).

Shouldn't this to be a reason to avoid Increment at all?!

No. Just because each operation (fetch/store) is atomic doesn't mean
that an increment (fetch/increment/store) is atomic:

counter=0

Thread 1 Thread 2
Fetch counter(0)
Fetch counter(0)
Increment (1)
Store (1)
Increment (1)
Store (1)

The result of two increments should be 2, but it's only 1.

Furthermore, atomic doesn't imply volatile - a fetch could see an old
value. See
http://www.pobox.com/~skeet/csharp/threads/volatility.shtml for more
information.