Java Threads "synchronized"

Question

This article talks about Java's "synchronized" keyword.

  ...
  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.

Now, my question:

Why doesn't "synchronized" on setFoo() prevent the above bolded line?

Samuel · Accepted Answer · 2013-02-27 19:04:51Z

6

This is an example of a check-then-act race condition.

A scenario might happen like the following:

Thread-1 getFoo() returns 0
Thread-2 getFoo() returns 0
Thread-2 setFoo(1)
Thread-1 setFoo(1)

This would mean that two threads have attempted to increment foo but it has the effect of only being incremented once.

As other answers have identified, synchronizing the incrementation with a synchronized block locking on the same object as getFoo() and setFoo() will prevent this race condition because threads won't be able to interleave like above.

edited Feb 27, 2013 at 19:04

answered Feb 27, 2013 at 17:53

Samuel

17.3k6 gold badges65 silver badges77 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

radai · Accepted Answer · 2013-02-27 17:52:29Z

6

because youre guaranteed no one else is getting foo alongside you, and that no one else is setting foo back besides you, but youre NOT guaranteed no one managed to get in and out (or just in) between you calling get() and you calling set()

you can think of that code as completely equivalent to this:

int temp = getFoo(); //safe method
temp = temp+1; //not protected here - im not holding any locks ...
setFoo(temp); //safe method

answered Feb 27, 2013 at 17:52

radai

24.3k10 gold badges76 silver badges118 bronze badges

Comments

rgettman · Accepted Answer · 2013-02-27 18:00:32Z

4

The synchronized keyword on both methods doesn't make it thread safe, because one thread could call getFoo, then another thread could call getFoo, and each of them get the same result. Then each of them add one and call setFoo, and the end result is that foo gets incremented only once, instead of twice. As your article points out, this is a race condition.

To make it thread safe, both the read and the write must be together in the same synchronized block, without separate get and set methods.

public synchronized void addFoo(int addend)
{
   foo += addend;
}

edited Feb 27, 2013 at 18:00

answered Feb 27, 2013 at 17:52

rgettman

179k30 gold badges282 silver badges365 bronze badges

4 Comments

Kevin Meredith Over a year ago

Then each of them add one and call setFoo, and the end result is that foo gets incremented only once, instead of twice Why?

rgettman Over a year ago

Because each Thread updates foo to the same value. For example, each of them gets the value 2, each of them adds 1 to get 3, then each of them sets the value to 3.

Kevin Meredith Over a year ago

If I understand right, then shouldn't your statement be Then each of them add one and call setFoo, and the end result is that foo gets incremented **twice**?

rgettman Over a year ago

It's a matter of semantics now. Sure, both threads increment foo, so it is incremented "twice". But each time, each thread increments it to 3, instead of one thread incrementing it to 3 and then the other thread incrementing it to 4.

Pshemo · Accepted Answer · 2013-02-27 18:31:20Z

Main trap in your code is that it may seem that getFoo will be called "inside" setFoo. Kind of

setFoo(){
   //getFoo();
   //...
}

which is incorrect because in reality getFoo is called before calling setFoo. Here is example that shows it:

public static int foo(int i) {
    System.out.print("FOO!");
    return i;
}

public static int bar(int i) {
    System.out.print("BAR!");
    return i;
}

public static void main(String[] args) throws Exception {
    System.out.println(foo(bar(1)));
}

Output:

BAR!FOO!1

As you can see bar was invoked before foo. So in your case it is possible that two (or more) threads will invoke getFoo which will return current value before they will call setFoo. In this case they will both have same value, lets say 0 and when they will call setFoo they will both set it to 1.

Andre · Accepted Answer · 2013-02-27 18:18:10Z

0

cant you use

   private volatile int foo;

or atomic http://docs.oracle.com/javase/tutorial/essential/concurrency/atomicvars.html

answered Feb 27, 2013 at 18:18

Andre

1622 silver badges8 bronze badges

1 Comment

Pshemo Over a year ago

volatile wont solve it, but atomic incrementation is good advice

Dustin Getz · Accepted Answer · 2013-02-27 18:54:30Z

0

does this code help?

class C {
  private int foo;
  public int getFoo() { return foo; } 
  public void setFoo(int f) { foo = f; }
}

C myC = new C();
synchronized(myC) {
  int foo = myC.getFoo();
  myC.setFoo(foo + 1);
}
println(myC.foo);

answered Feb 27, 2013 at 18:54

Dustin Getz

22k15 gold badges87 silver badges132 bronze badges

1 Comment

Kevin Meredith Over a year ago

So the point is that myC.setFoo(1) could get called before println(myC.getFoo())?

Collectives™ on Stack Overflow

Java Threads "synchronized"

6 Answers 6

Comments

Comments

4 Comments

Comments

1 Comment

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

6 Answers 6

Comments

Comments

4 Comments

Comments

1 Comment

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related