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

On Apr 30, 2:38=A0pm, Philipp <sicsic...@freesurf.ch> wrote:

> ...
> 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 {
> =A0 =A0 =A0private int value =3D 0;
>
> =A0 =A0 =A0public int getValue() {
> =A0 =A0 =A0 =A0 =A0return value;
> =A0 =A0 =A0}
> =A0 =A0 =A0public void setValue(int value) {
> =A0 =A0 =A0 =A0 =A0this.value =3D value;
> =A0 =A0 =A0}
> }

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 {
> =A0 =A0 =A0private volatile long value =3D 0;
>
> =A0 =A0 =A0public long getValue() {
> =A0 =A0 =A0 =A0 =A0return value;
> =A0 =A0 =A0}
> =A0 =A0 =A0public void setValue(long value) {
> =A0 =A0 =A0 =A0 =A0this.value =3D value;
> =A0 =A0 =A0}
> }

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

"Philipp" <sicsicsic@freesurf.ch> wrote in message 
news:1209559089_4017@sicinfo3.epfl.ch...
> 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)

I find the best summary to be Doug Lea's "Concurrent Programming..." p. 
92-97, which he says he takes from the JLS chapter 17 on the memory model. 
My 2nd edition (C) 2000 copy says that this section is still being updated 
for Java 2, so it is likely to be reasonably "authoritative" for 1.3. 
However, he points out (and I have read from others) that the early memory 
model is unclear in some places (p. 95 footnote) and downright broken in 
others.  And regardless of what is authoritatively written, any given JVM of 
that vintage may or may not be implemented perfectly.

>
> 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;
>     }
> }

According to Paul Hyde's Java Thread Programming p. 127, "Before Sun 
included JIT with their VMs, volatile did not make a difference."

Nevertheless, if this code is accessed from threads that are not otherwise 
synchronized, the lack of volatile may cause setValues in thread to not be 
visible to another. The volatile keyword is sufficient to fix that because 
the accesses and updates to the value are independent.  If the code had 
expressions such as value = value * 4 or value++, these must be sychronized 
because they will appear as two or more independent operations. As long as 
the access or updates are recognized to be totally independent, volatile is 
acceptable.

>
>
> // 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;
>     }
> }

Doug Lea also asserts on p.93 that "atomicity extends to volatilie long and 
double" even though they are not atomic when not volatile.

The next two cases are similar.  They are threadsafe only to the extent that 
the object reference is being updated in a timely and atomic way.  There is 
no guarantee of timely update for any fields of the assigned String or Date 
and only their initial value will be visible.  I'm not sure what counts as 
"initial value" in this case, being that String and Date both assign most of 
their fields in the constructor. This may be the uncertainty in the JLS that 
Doug Lea talks about on p. 95.

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.

Matt Humphrey http://www.iviz.com/ 

Matt Humphrey wrote:
>
> 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). 

"Mike Schilling" <mscottschilling@hotmail.com> wrote in message 
news:cF4Sj.457$J16.156@newssvr23.news.prodigy.net...
> Matt Humphrey wrote:
>>
>> 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).

I agree, but this raises to my mind a case whether synchronized is necessary 
or not.  The case I'm thinking of is like this:

class Factorization {
  private int [] factors;

  public Factorization (int value) {
    factors = new int [20];
    computeFactors (value);
  }

  public void computeFactors (int value) {
    // Internal code that populates the array
  }

  public int getTotal () {
    if (factors == null) return 0;
    int total = 0;
    for (int i =0; i < factors.length; ++i) {
     total += factors[i];
    }
    return total;
}

class MathBox {
  private volatile Factorization factorization;
  public Factorization getF () { return factorization; }
  public void setF (Factorization f) { factorization = f; }
}

static MathBox sharedMathBox = new MathBox ();

// Writer thread
for (int i = 0; i < 100; ++i) {
  Factorization factorization = new Factorization (i);
  // Alternate position for computeFactors ()
  sharedMathBox.setF (factorization);
}

// Reader thread
for (int i = 0; i < 100; ++i) {
  Factorization factorization = sharedMathBox.getF ();
  System.out.println ("" + factorization.getTotal ());
}

Despite the contrivance of 2 totally unsynchronized threads, what is not 
clear to me is what the read thread will see for the values for the 
factorization.  The volatile keyword ensures that the factorization 
reference arrives safely, but it doesn't guarantee the fields.  The fields 
are assigned within the execution of the constructor, but I don't think 
that's sufficient; I don't think it counts as the visibility of the initial 
value.

Will the reader sometimes see a total of 0 because factors is null?  Will it 
sometimes see an inconsistent result because not all of the values have been 
updated?  Or will it always be up-to-date because it falls within the 
natural ordering of the constructor?

It does seem certain that if computeFactors is moved out of the constructor 
and into the loop at the marked position that the reader thread may 
sometimes see inconsistent results.  Synchronizing computeFactors would 
ensure that the reader always sees the up-to-date values.

I tested volatile on a 2-processor hyperthreaded system a couple years ago 
and I could not get non-volatile values to fail.  I would like to know what 
the correct model is for this case, but I would only trust an empirical test 
that actually showed a failure.  (The test might not fail because of CPU 
caching, JIT performance, etc.)

Matt Humphrey http://www.iviz.com/ 

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.



On Apr 30, 11:56=A0am, Szabolcs Ferenczi <szabolcs.feren...@gmail.com>
wrote:
> On Apr 30, 2:38=A0pm, Philipp <sicsic...@freesurf.ch> wrote:
>
> > ...
> > 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 {
> > =A0 =A0 =A0private int value =3D 0;
>
> > =A0 =A0 =A0public int getValue() {
> > =A0 =A0 =A0 =A0 =A0return value;
> > =A0 =A0 =A0}
> > =A0 =A0 =A0public void setValue(int value) {
> > =A0 =A0 =A0 =A0 =A0this.value =3D value;
> > =A0 =A0 =A0}
> > }
>
> 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 {
> > =A0 =A0 =A0private volatile long value =3D 0;
>
> > =A0 =A0 =A0public long getValue() {
> > =A0 =A0 =A0 =A0 =A0return value;
> > =A0 =A0 =A0}
> > =A0 =A0 =A0public void setValue(long value) {
> > =A0 =A0 =A0 =A0 =A0this.value =3D value;
> > =A0 =A0 =A0}
> > }
>
> Atomicity and volatileness are two different issues: "Locking can
> guarantee both visibility and atomicity; volatile variables can only
> guarantee visibility." (Java Concurrency in Practice, ByBrian 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.

brian@briangoetz.com wrote:
> 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

Philipp wrote:
> brian@briangoetz.com wrote:
>> 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 general yes, unless updates are only happening on a single thread.

Mark Thornton

Philipp wrote:
> brian@briangoetz.com wrote:
>> 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?
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.
-- 
Daniel Pitts' Tech Blog: <http://virtualinfinity.net/wordpress/>

Philipp wrote:

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

Use AtomicInteger

-- 

"I'm a doctor, not a mechanic." Dr Leonard McCoy <mccoy@ncc1701.starfleet.fed>
"I'm a mechanic, not a doctor." Volker Borchert  <v_borchert@despammed.com>