CODEWITHSUNDEEP
javasynchronizationthread-safety
B M
B M

Reputation: 671

What synchronization issue does the following JAVA code suffer from?

The author of this page mentions that the second code example below suffers from a synchronization problem but the code will work about 99 times in 100. I am not able to see the operational difference in the two programs in that page.

A somewhat more general solution that combines the previous two is to copy the value of the field into a local variable, then change only the local variable. The field remains unchanged inside the method. For example,

public class Counter {

  int count = 0;

  public void count() {
    int count = this.count;
    int limit = count + 100;
    while (count++ != limit) System.out.println(count); 
  }

}

Note how the local variable count shadows the field count, and how the this keyword is used to refer to the field count outside the shadow.

This trick is primarily useful when you don't need to save the changed variable back into the field when the method is done. The following saves the state, but is still subject to less obvious synchronization problems.

public class Counter {

  private int count = 0;

  public void count() {
    int count = this.count;
    int limit = count + 100;
    while (count++ != limit) System.out.println(count);
    this.count = count; 
  }

}

In fact, this is probably even worse than the original example because it will work 99 times out of a 100. The bug here is extremely hard to pin down if you don't spot it in the source code.

Upvotes: 2

Views: 78

Answers (1)

paxdiablo
paxdiablo

Reputation: 881553

The problem lies with the fact that the modification to this.count is not atomic.

Let's say two threads are calling this function and this.count is currently set to 0.

Thread 1 loads 0 into its local count and then starts incrementing it to 101 (not 100 as previously thought since it increments count one more time after the limit is reached, with count++ != limit).

In the meantime, thread 2 comes in and this.count is still set to 0 so it grabs it and starts incrementing to 101.

At some point, they then both reach the line this.count = count (and the order doesn't matter) and they both set this.count to 101.

What it should have been set to at the end was 202 (I think - certainly it should be around the 200 mark somewhere).

The reason the first bit of code is valid is because it's not actually trying to change the this.count value hence no race condition is possible.

Upvotes: 2

Related Questions