Thread safety of readonly members

M

Michi Henning

Hi, I can't find a statement about this in the threading
sections in the doc...

Consider:

class Class1
{
Class1()
{
_val = 42;
}

public int getVal()
{
return _val;
}

private readonly int _val;
}

In a threaded environment, is it necessary to interlock
inside the constructor and getVal() to ensure that
threads get the correct value? In other words,
do I have to write it as follows?

class Class1
{
Class1()
{
lock(this)
{
_val = 42;
}
}

public int getVal()
{
lock(this)
{
return _val;
}
}

private readonly int _val;
}

In C++, the lock is necessary because, on SMP
machines, memory consistency isn't guaranteed without
the lock. But I don't know whether the same is true
for C#.

What if the member variable is not readonly (but will
never be modified, except for the initial assignment
in the constructor)? Is accessing the value thread-safe
without a lock in that case?

What about const members? Is access to those safe without
a lock?

And what about static members that are (conceptually)
immutable? Is the lock required there?

Thanks muchly,

Michi.
 
M

Michael Sparks

Michi Henning said:
In a threaded environment, is it necessary to interlock
inside the constructor and getVal() to ensure that
threads get the correct value? In other words,
do I have to write it as follows?

In the specific example you gave, it is not necessary, for a few reasons.

Since all you have is the constructor and getVal(), there is never any
competition for the shared resource. When the object is being constructed,
nobody has a reference to it yet, so nobody can call getVal(). You can be
sure that anybody calling getVal() will be calling it after the constructor
has finished executing.

The data can never be in an inconsistent state since it is just a single
int. Ints are assigned atomically. I'm not sure about other types - I am
sure that structs are not assigned atomically, but I'm not positive about
numbers that are > 32 bits (I suspect they are not atomic - perhaps someone
can clear this up). Suppose we add a putVal() method to complicate things.
Suppose that someone calls putVal(0) around the same time someone calls
getVal(). What will they get? Well, they will either get 42 or 0. A lock
doesn't change that.
In C++, the lock is necessary because, on SMP
machines, memory consistency isn't guaranteed without
the lock. But I don't know whether the same is true
for C#.

In the cases where this is true of C++, it is also true of C#. Your
specific example doesn't require synchronization in either language, but I
realize you were probably just trying to simplify things for your post.
What if the member variable is not readonly (but will
never be modified, except for the initial assignment
in the constructor)? Is accessing the value thread-safe
without a lock in that case?
Yes.

What about const members? Is access to those safe without
a lock?
Yes.

And what about static members that are (conceptually)
immutable? Is the lock required there?

No lock required in this case either.

I'm not sure from your post if you have a basic understanding of
multithreaded programming. The principles are the same in most languages.
If you are unsure, I'd suggest doing some reading on that topic.

HTH
Mike
 
R

Richard A. Lowe

Re:; the atmomicity of types. Section 5.5 of the spec reads:

5.5 Atomicity of variable references

Reads and writes of the following data types are atomic: bool, char, byte,
sbyte, short, ushort, uint, int, float, and reference types. In addition,
reads and writes of enum types with an underlying type in the previous list
are also atomic. Reads and writes of other types, including long, ulong,
double, and decimal, as well as user-defined types, are not guaranteed to be
atomic. Aside from the library functions designed for that purpose, there is
no guarantee of atomic read-modify-write, such as in the case of increment
or decrement.

Richard
 
J

Jon Skeet [C# MVP]

Michael Sparks said:
In the specific example you gave, it is not necessary, for a few reasons.

Since all you have is the constructor and getVal(), there is never any
competition for the shared resource. When the object is being constructed,
nobody has a reference to it yet, so nobody can call getVal(). You can be
sure that anybody calling getVal() will be calling it after the constructor
has finished executing.

That's not quite true, actually. Unfortunately, compiler memory
reordering could end up with the pseudocode of:

Thread 1:
o Create new "empty" object
o Store reference
o Call constructor

A second thread could come in between the second and third parts, and
call getVal on the reference.

Basically, unless you've got memory fences involved, I believe the
compiler is free to do whatever it likes in terms of reordering, so
long as each individual thread sees something consistent with itself.

It's only theoretical, and I strongly suspect that it would never
happen on any current CLR implementation, but I believe it *is* a
potential problem. Effectively, the issue is how the reference itself
is visible - if the reference is assigned in a lock (and the same lock
is required to access it for the first time), all would be well.

Of course, I could be entirely wrong about this, but that's the way I
understand it...
 
M

Michael Sparks

Jon Skeet said:
Thread 1:
o Create new "empty" object
o Store reference
o Call constructor

A second thread could come in between the second and third parts, and
call getVal on the reference.

Basically, unless you've got memory fences involved, I believe the
compiler is free to do whatever it likes in terms of reordering, so
long as each individual thread sees something consistent with itself.

Aaah. I was making an assumption based on the newobj IL opcode, which
should behave (I think) as I stated. I didn't think about the JIT
reordering things.
Learn something new every day. :)

Mike
 
M

Michi Henning

Michael Sparks said:
In the specific example you gave, it is not necessary, for a few reasons.

Since all you have is the constructor and getVal(), there is never any
competition for the shared resource. When the object is being constructed,
nobody has a reference to it yet, so nobody can call getVal(). You can be
sure that anybody calling getVal() will be calling it after the constructor
has finished executing.

That's not the problem. The question is whether it is possible for another
thread to read a stale value of the memory location that holds _val some
time after the constructor completes. On SMP machines, if no memory
barrier is inserted after the initialization, another thread can read a stale
value out of its cache. So, I guess another way to formulate my questions
is to ask whether C# sharp guarantees to insert memory barriers after
initializating readonly data members.
The data can never be in an inconsistent state since it is just a single
int. Ints are assigned atomically.

Again, that's not relevant. The issues isn't whether the value is assigned
to atomically, but whether the lock is required to guarantee that
memory is consistent across different threads after the initialization.
Suppose we add a putVal() method to complicate things.
Suppose that someone calls putVal(0) around the same time someone calls
getVal(). What will they get? Well, they will either get 42 or 0. A lock
doesn't change that.

Uh, no -- if there is no memory barrier, another thread might read 99,
if that is the value that was in the corresponding memory location
last time that thread read it.
In the cases where this is true of C++, it is also true of C#. Your
specific example doesn't require synchronization in either language, but I
realize you were probably just trying to simplify things for your post.

Sorry, no -- in C++, you definitely need the lock. That's because
C++ doesn't make any promises as to memory consistency in the
absence of locks. I've seen the equivalent C++ actually blow up
on Alpha SMP machines, for example.
I'm not sure from your post if you have a basic understanding of
multithreaded programming. The principles are the same in most languages.
If you are unsure, I'd suggest doing some reading on that topic.

Uh, as one of the implementers of POSIX Draft 4 UNIX kernel
threads back in 1990, I think I have a passing familiarity with
the topic ;-)

Cheers,

Michi.
 
M

Michi Henning

Richard A. Lowe said:
Re:; the atmomicity of types. Section 5.5 of the spec reads:

5.5 Atomicity of variable references

Reads and writes of the following data types are atomic: bool, char, byte,
sbyte, short, ushort, uint, int, float, and reference types. In addition,
reads and writes of enum types with an underlying type in the previous list
are also atomic. Reads and writes of other types, including long, ulong,
double, and decimal, as well as user-defined types, are not guaranteed to be
atomic. Aside from the library functions designed for that purpose, there is
no guarantee of atomic read-modify-write, such as in the case of increment
or decrement.

Right, but this doesn't apply to my question, which really is about memory
consistency, not atomicity.

But that raises an interesting question: why bother providing the atomicity
guarantee in the language at all? The guarantee would appear to be useless
in the absence of locks. But, in the presence of locks, the guarantee would
appear to be unnecessary?

Cheers,

Michi.
 
M

Michi Henning

Jon Skeet said:
That's not quite true, actually. Unfortunately, compiler memory
reordering could end up with the pseudocode of:

Thread 1:
o Create new "empty" object
o Store reference
o Call constructor

A second thread could come in between the second and third parts, and
call getVal on the reference.

Basically, unless you've got memory fences involved, I believe the
compiler is free to do whatever it likes in terms of reordering, so
long as each individual thread sees something consistent with itself.

Right -- that's exactly what I was getting at. So, no guarantees in
the language then about readonly member initialization. Thanks
for clarifying this Jon!
It's only theoretical, and I strongly suspect that it would never
happen on any current CLR implementation, but I believe it *is* a
potential problem. Effectively, the issue is how the reference itself
is visible - if the reference is assigned in a lock (and the same lock
is required to access it for the first time), all would be well.

Right, it wouldn't happen on an Intel processor. But
I know that the race is real on some architectures. For example,
double-checked locking blows up on Alpha SMP machines because
the memory model is more liberal than that of an Intel processor.
So, the code I showed could break if run on such a machine,
for the same reason.

Cheers,

Michi.
 
S

Sherif ElMetainy

Hello
It's only theoretical, and I strongly suspect that it would never
happen on any current CLR implementation, but I believe it *is* a
potential problem. Effectively, the issue is how the reference itself
is visible - if the reference is assigned in a lock (and the same lock
is required to access it for the first time), all would be well.

I don't know if such a problem exists. Personally, I think it would be a CLR
bug if such a problem exist in any CLR implementation, because this would
allow access to an object without the constructor being called, which
destroys the concept of having a contructor. This can cause a lot of bugs
and security holes in programs. Also, the performance loss caused by the
need to make a lock on constructor calls, would be more than the performance
gain by such reordering.

Best regards,
Sherif
 
M

Michi Henning

Sherif ElMetainy said:
Hello


I don't know if such a problem exists. Personally, I think it would be a CLR
bug if such a problem exist in any CLR implementation, because this would
allow access to an object without the constructor being called, which
destroys the concept of having a contructor.

No, I don't think that can happen. Some time after a thread has
allocated the object, it has to pass the reference to another thread
somehow. Of course, that has to happen either as a method parameter
(in which case there is no locking issue), or the reference is passed
by assigning it to some variable that is read by another thread. In the
latter case, the thread that writes the reference must of course interlock
with the thread that reads the reference.

The point of the example I showed is not about passing the reference
in a thread-safe way, but about accessing the contents of the object
pointed at by the reference. From the discussion, it appears that,
without acquiring a lock in the constructor and in getVal(), there
is no guarantee that another thread will read the correct value,
not withstanding the fact that the constructor has finished executing.

I got a reference to the following in the Mono development list,
which explains it nicely (and confirms that the locks are required):

http://research.microsoft.com/~birrell/papers/ThreadsCSharp.pdf

Cheers,

Michi.
 
M

Michael Sparks

Michi Henning said:
That's not the problem. The question is whether it is possible for another
thread to read a stale value of the memory location that holds _val some
time after the constructor completes. On SMP machines, if no memory
barrier is inserted after the initialization, another thread can read a stale
value out of its cache. So, I guess another way to formulate my questions
is to ask whether C# sharp guarantees to insert memory barriers after
initializating readonly data members.

Sorry, I thought you were trying to use mutual exclusion to maintain an
invariant on the int, which of course isn't necessary.
Uh, as one of the implementers of POSIX Draft 4 UNIX kernel
threads back in 1990, I think I have a passing familiarity with
the topic ;-)

My apologies if I have insulted your manhood. :)

Mike
 
S

Sherif ElMetainy

Hello Michi
Thanks for the link for the document, it is great. One issue the author
discusses in section 4.3, here is his sample code

Foo theFoo = null;
public Foo GetTheFoo() {
if (theFoo == null) {
lock (this) {
if (theFoo == null)
theFoo = new Foo();
}
}
return theFoo;
}
The author says the above code may cause race conditions. I quote the Author
now
Start Quote "

First, if "GetTheFoo" is called on processor A and then later on processor
B, it's possible that processor B will see the correct non-null object
reference for "theFoo", but will read incorrectly cached values of the
instance variables inside "theFoo", because they arrived in B's cache at
some earlier time, being in the same cache line as some other variable that'
s cached on B. *
Second, it's legitimate for the compiler to re-order statements within a
"lock" statement, if the compiler can prove that they don't interfere.
Consider what might happen if the compiler makes the initialization code for
"new Foo()" be inline, and then re-orders things so that the assignment to
"theFoo" happens before the initialization of the instance variable's of
"theFoo". A thread running concurrently on another processor might then see
a non-null "theFoo" before the object instance is properly initialized.
"
End Quote

I don't quite understand the first point, I understand the second issue,
which is similar to what Jon has mentioned. But a look at the source code of
some of the .NET classes shows that Microsoft makes this mistake (if this is
a mistake) at many occasions

for example below is the code of a method called InitResourceManager in
System.Environment class
private static ResourceManager InitResourceManager() {
if (SystemResMgr == null) {
lock(typeof(Environment)) {
if (SystemResMgr == null) {
// Do not reorder these two field assignments.
m_resMgrLockObject = new Object();
SystemResMgr = new ResourceManager("mscorlib",
typeof(String).Assembly);
}
}
}
return SystemResMgr;
}


The above code is basically the same as the document's author faulty code,
yet it is extracted from the .NET (Rotor) source code.
So is there anywhere in .NET's documentation and specifications something
that mentions whether or not it is gauranteed that an object's constructor
must be called before any thread is allowed to access it? Or this assignment
/ constructor reordering? This is a very important question for me, because
I have been following Microsoft's example above in a lot my applications,
and it would be a problem for me if this doesn't work.

Best regards,
Sherif
 
J

Jon Skeet [C# MVP]

The above code is basically the same as the document's author faulty code,
yet it is extracted from the .NET (Rotor) source code.
So is there anywhere in .NET's documentation and specifications something
that mentions whether or not it is gauranteed that an object's constructor
must be called before any thread is allowed to access it? Or this assignment
/ constructor reordering? This is a very important question for me, because
I have been following Microsoft's example above in a lot my applications,
and it would be a problem for me if this doesn't work.

It *might* work, but it's not good to rely on it, because it's not
guaranteed to work. The thing to read is the section on memory
guarantees in the CLR spec.

Of course, the Rotor project may decide to use technically dodgy code
in its own BCL if it absolutely knows that its own VES *doesn't* make
it problematic. Still not a good idea though, IMO...
 
M

Michi Henning

I don't quite understand the first point, I understand the second issue,
which is similar to what Jon has mentioned.

The first issue is about how CPU memory caches work. If a thread
reads, for example, a four-byte integer at address 0x10 from
physical memory, the underlying hardware will read a cache
line. Cache lines typically contain several words of memory (16 or 32
bytes or more). So, the access of four bytes at 0x10 causes more
memory to be cached by the CPU, for example, all the memory
contents from 0x10 to 0x2F (assuming that a cache line is 32 bytes
long).

So, assume that thread 1 at some time in the past has read a variable
at address 0x10. That means that the contents of 0x10-0x2F were read into
the CPU cache of thread 1 at that time. Some time later, thread 2 runs
on a different CPU and allocates the Class1 instance. It just so happens
that the instance ends up at address 0x14. Thread 2 also runs the
constructor of the instance.

Some time later, thread 1 calls getVal() on that instance. The CPU of
thread 1 realizes that it has the contents of address 0x14 already in its cache
and
satisfies the read from the cache, not knowing that the contents of
that memory location were previously modified by thread 2, so thread 1
reads a garbage value. With the locks in place, this can't happen because
that causes the compiler to insert instructions that ensure memory
consistency.
The above code is basically the same as the document's author faulty code,
yet it is extracted from the .NET (Rotor) source code.

On an x86 machine, double-checked locking happens to work. But that's
not true for other architectures. In particular, double-checked locking fails
on Alphas. So, the code is non-portable.

Cheers,

Michi.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Top