Thread safety and atomic assignment (again)

[Philipp]

Hello,
By searching the web I found lots of references about the below
question. At the same time, I couldn't find an *authoritative*
references (eg. by Goetz, Lea or Bloch) about it. Please refer me to the
correct web site or book. Thanks (I'm using JVM 1.3, so the "old" memory
model)

The question: Are the following code snippets thread safe and why? (my
opinion is on top, please correct if wrong):

// Not thread safe. "value" needs to be volatile, else another thread
// might see a stale value
public final class Test1 {
private int value = 0;

public int getValue() {
return value;
}
public void setValue(int value) {
this.value = value;
}
}


// Not thread safe. Because assignment to long is not atomic (although
// volatile).
public final class Test2 {
private volatile long value = 0;

public long getValue() {
return value;
}
public void setValue(long value) {
this.value = value;
}
}


// Thread safe. Because reference assignment is atomic
// Question: Has immutability of String any influence here?
// Question: Could "newValue" be incompletely constructed?
public final class Test3 {
private volatile String value = "Hello";

public String getValue() {
return value;
}
public void setValue(String newValue) {
this.value = newValue;
}
}

// Not thread safe. Because construction may not have finished when
// reference assignment is made (although volatile).
public final class Test4 {
private volatile Date value = new Date();

public Date getValue() {
return value;
}
public void makeNewDate() {
this.value = new Date();
}
}

// (Still) not thread safe. Because 2 step assignement makes no
// guaranteeing that the ref will not be assigned earlier.
public final class Test5 {
private volatile Date value = new Date();

public Date getValue() {
return value;
}
public void makeNewDate() {
Date d = new Date();
this.value = d;
}
}


Thanks for your answers and clarifications.

Phil

 

[Szabolcs Ferenczi]

...
The question: Are the following code snippets thread safe and why? (my
opinion is on top, please correct if wrong):

What is your definition of thread safeness in the case of your
examples?

// Not thread safe. "value" needs to be volatile, else another thread
// might see a stale value
public final class Test1 {
     private int value = 0;

     public int getValue() {
         return value;
     }
     public void setValue(int value) {
         this.value = value;
     }
}

This construction is regarded not to be thread safe. However, with
volatile int, you only solve the so-called visibility problem but I
would not regard the whole class thread safe even with the volatile
int. Here the question again what do you regard to be a stale value.

// Not thread safe. Because assignment to long is not atomic (although
// volatile).
public final class Test2 {
     private volatile long value = 0;

     public long getValue() {
         return value;
     }
     public void setValue(long value) {
         this.value = value;
     }
}

Atomicity and volatileness are two different issues: "Locking can
guarantee both visibility and atomicity; volatile variables can only
guarantee visibility." (Java Concurrency in Practice, By Brian Goetz,
Tim Peierls, Joshua Bloch, Joseph Bowbeer, David Holmes, Doug Lea)

As far as I know, however, single read or single write is atomic in
case of volatile long, so it should be thread safe in your
interpretation of thread safeness.

I think the rest of the examples, Test3-5 are all unsafe because of
the same reason: Atomicity and volatileness are two different issues.
In those cases you require atomicity to make it thread safe.

Best Regards,
Szabolcs

 

[Mike Schilling]

As a clearer example of this point, if the object used for setValue
had non-volatile fields that were not assigned in the constructor,
there is nothing that guarantees that the updated values of those
fields will be visible in a different thread.

Unless, of coujrse, the object itself guarantees that by synchronizing
the getter(s) and setter(s).

 

[brian]

Perhaps I can clarify. Szabolics' analysis is correct, but I would
put it slightly differently. Its not so much a question of "what do
you mean by safe." Its a question of what you expect from this
class.

By making the internal field volatile, you have made your class data-
race free. This means that callers of get() will see the most recent
value passed by any thread to set(). However, you cannot use this
class to safely build a counter, for example; uses like

foo.set(foo.get() + 1)

are in danger of "lost updates" because there's no way to make the
set(get()+1) operation atomic with respect to other ops on Test2
object. That might be OK.

Another way to put this is that while this class is safe, it can be
used in a not-thread-safe manner, and that doesn't make the class not
safe, it makes the use not safe.

 

[Philipp]

Perhaps I can clarify. Szabolics' analysis is correct, but I would
put it slightly differently. Its not so much a question of "what do
you mean by safe." Its a question of what you expect from this
class.

By making the internal field volatile, you have made your class data-
race free. This means that callers of get() will see the most recent
value passed by any thread to set(). However, you cannot use this
class to safely build a counter, for example; uses like

foo.set(foo.get() + 1)

are in danger of "lost updates" because there's no way to make the
set(get()+1) operation atomic with respect to other ops on Test2
object. That might be OK.

If I understand this correctly, then synchronizing set and get would
guarantee the correct counter behavior by making set and get *mutually*
exclusive, which is not guaranteed by volatile.
But is there a difference between the above and this?

int newValue = foo.get() + 1;
foo.set(newValue);

Does the call in one single line guarantee that the lock is passed
directly from the get to the set? Else, another thread with another get
might interfer.

Can a counter only be implemented with a specific synchronized
"increment()" method?

In summary, how can you guys sleep at night without making *everything*
synchronized? (and at the same time, having nightmares about deadlocks I
guess)... ?

Phil
PS: Thanks for the references to Matt Humphrey and Szabolcs

 

[Mark Thornton]

If I understand this correctly, then synchronizing set and get would
guarantee the correct counter behavior by making set and get *mutually*
exclusive, which is not guaranteed by volatile.
But is there a difference between the above and this?

int newValue = foo.get() + 1;
foo.set(newValue);

Does the call in one single line guarantee that the lock is passed
directly from the get to the set? Else, another thread with another get
might interfer.

Can a counter only be implemented with a specific synchronized
"increment()" method?

In general yes, unless updates are only happening on a single thread.

Mark Thornton

 

[Daniel Pitts]

If I understand this correctly, then synchronizing set and get would
guarantee the correct counter behavior by making set and get *mutually*
exclusive, which is not guaranteed by volatile.
But is there a difference between the above and this?

Actually, that isn't any different than volatile really.
You have to synchronize the entire operation.
Imagine two threads A and B, and foo's value is 10
A: foo.get() is called, returns 10
B: foo.get() is called, returns 10
A: adds one to result (11) and stores it in newValue.
B: adds one to result (11) and stores it in newValue.
A: foo.set(11) gets called
B: foo.set(11) gets called.

Whoops, we just lost a count! What you need to do to ensure your
counter works as expected is to synchronize the "increment" operation.

int newValue = foo.get() + 1;
foo.set(newValue);

Does the call in one single line guarantee that the lock is passed
directly from the get to the set? Else, another thread with another get
might interfer.

Exactly! The lock isn't passed on.

Can a counter only be implemented with a specific synchronized
"increment()" method?

Man, I should have read you're entire post before replying, you've got
the right idea.

In summary, how can you guys sleep at night without making *everything*
synchronized? (and at the same time, having nightmares about deadlocks I
guess)... ?

I sleep fine by knowing what must be an atomic operation, and what must
be thread-safe, and what doesn't need to be one or the other. I tend to
prefer Thread Confinement, such as having a separate Context per thread,
so mutable objects aren't passed to other threads. That approach is
useful in servlets. Thread Confinement is also used in AWT/Swing on the
Event Dispatch Thread.

Phil
PS: Thanks for the references to Matt Humphrey and Szabolcs

I suggest reading the book Java Concurrency in Practice by Doug Lea. It
is an excellent reference on what, why, and how in handle multi-threaded
programming. Its not too thick, and one of the few reference books
worth ready cover to cover.

 

[Volker Borchert]

Philipp wrote:

|> Can a counter only be implemented with a specific synchronized
|> "increment()" method?

Use AtomicInteger