synchronized confusion

[Zhao]

Here is the issue I want to discuss

"
....
private int foo;
public synchronized int getFoo() { return foo; }
public synchronized void setFoo(int f) { foo = f; }

If a caller wants to increment the foo property, the following code to
do so is not thread-safe:

...
setFoo(getFoo() + 1);

If two threads attempt to increment foo at the same time, the result
might be that the value of foo gets increased by one or by two,
depending on timing.
....
"

I couldn't figure out why it not thread-safe...

Any advice will be appreciated.

thanks

 

[Kristian Bisgaard Lassen]

Here is the issue I want to discuss

"
...
private int foo;
public synchronized int getFoo() { return foo; }
public synchronized void setFoo(int f) { foo = f; }

If a caller wants to increment the foo property, the following code to
do so is not thread-safe:

...
setFoo(getFoo() + 1);

If two threads attempt to increment foo at the same time, the result
might be that the value of foo gets increased by one or by two,
depending on timing.
...
"

I couldn't figure out why it not thread-safe...

Any advice will be appreciated.

thanks

Hi,

It is thread unsafe since it is undeterministic.
Consider two examples where foo gets a different value. I us threads A and B and foo = 0:

A B
setFoo (getFoo () + 1) setFoo (getFoo () + 1)
setFoo (0 + 1) setFoo (getFoo () + 1)
done... setFoo (1 + 1)
done...
foo = 2.

Now consider this:
A B
setFoo (getFoo () + 1) setFoo (getFoo () + 1)
setFoo (0 + 1) setFoo (getFoo () + 1)
setFoo (0 + 1) setFoo (0 + 1)
setFoo (0 + 1) done...
done..

foo = 1

In the latter example process A is much slower in the execution of the
statement but that is perfectly possible, if for example the scheduler
switched from thread A to thread B after getFoo ().

Best Regards
Kristian

 

[Shripathi Kamath]

Here is the issue I want to discuss

"
...
private int foo;
public synchronized int getFoo() { return foo; }
public synchronized void setFoo(int f) { foo = f; }

If a caller wants to increment the foo property, the following code to
do so is not thread-safe:

...
setFoo(getFoo() + 1);

If two threads attempt to increment foo at the same time, the result
might be that the value of foo gets increased by one or by two,
depending on timing.
...
"

I couldn't figure out why it not thread-safe...

That is because there is a gap between when you access the variable, and
when you modify it during which the variable might have changed.

Consider two threads who do the following:

setFoo(getFoo() + 1);



It is better understood if you consider the above in the proper light:

int a = getFoo();
setFoo(a + 1);


The two threads could end up doing the first part in order which might yield
what you think is a predictable result, ie.

Thread A: int a = getFoo();
Thread A: setFoo(a+1);
Thread B: int a = getFoo();
Thread B: setFoo(a+1);

or they may end up doing it
Thread A: int a = getFoo();
Thread B: int a = getFoo();
Thread A: setFoo(a+1);
Thread B: setFoo(a+1);

or
Thread A: int a = getFoo();
Thread B: int a = getFoo();
Thread B: setFoo(a+1);
Thread A: setFoo(a+1);

or something else.

In each the results will vary.

The synchronized construct applies to what happens inside the method, not
oustide it.

If you need to do the thing safely one way to do so would be to add a
synchronized method, something like:

synchronized void increment() {
++foo;
}


HTH,

 

[Thomas G. Marshall]

Here is the issue I want to discuss

"
...
private int foo;
public synchronized int getFoo() { return foo; }
public synchronized void setFoo(int f) { foo = f; }

If a caller wants to increment the foo property, the following code to
do so is not thread-safe:

...
setFoo(getFoo() + 1);
....[rip]...

I couldn't figure out why it not thread-safe...

You'll need:

synchronized void incFoo() { foo++); }

Because any one thread is doing this:

getFoo()
Add 1, store in x
setFoo(x)

These 3 operations are not mutex protected between them. The mutex occurs
within getFoo and within setFoo seperately...

Here, take a look at this quick example: The code at the below will try to
increment foo by 1 a million times, but in two concurrent threads.

This should result in foo incremented to 2,000,000, right?

But when incremented your way (see code), this is produced:

te.foo[1438404]

If I change the call to something safer (the synched addFoo(1) instead) you
get:

te.foo[2000000]

Like you expected.

------------------------------------[code
example]-----------------------------
public class ThreadExper
{
public int foo = 0;
public synchronized int getFoo() { return foo; }
public synchronized void setFoo(int f) { foo = f; }
public synchronized void addFoo(int inc) { foo += 1; }

public static void main(String[] args)
{
final ThreadExper te = new ThreadExper();

Runnable runnable = new Runnable()
{
public void run()
{
for (int i=0; i < 1000000; i++)
{
te.setFoo(te.getFoo() + 1);
//te.addFoo(1);
}
}
};

Thread t1, t2;

(t1=new Thread(runnable)).start();
(t2=new Thread(runnable)).start();

while (t1.isAlive() || t2.isAlive())
try {Thread.sleep(1000);} catch(Exception ignore) {}

System.out.println("te.foo["+te.foo+"]");

}
}