Multithreading ThreadSeven extends Thread

Question

I have simple question from OCJP

Given:

1. public class TestSeven extends Thread {
2.     private static int x;
3.     public synchronized void doThings() {
4.         int current = x;
5.         current++;
6.         x = current;
7.     }
8.     public void run() {
9.         doThings();
10.    }
11. }

Which statement is true?

A. Compilation fails.

B. An exception is thrown at runtime.

C. Synchronizing the run() method would make the class thread-safe.

D. The data in variable "x" are protected from concurrent access problems.

E. Declaring the doThings() method as static would make the class thread-safe.

F. Wrapping the statements within doThings() in a synchronized(new Object()) { } block would make the class thread-safe.

Answer is option E.

My Question: as method doThings() is already synchornozed, doesn't it make thread safe?

Please also give some good links for these topics.

Answer 1

The problem is that x is a static variable, that is thus shared by all the threads. And since all the threads are not synchronized on a single object (every thread uses this as the lock), nothing prevents two threads to execute the doThings() method in parallel. Two threads might thus read the value of x in parallel, and then increment it in parallel, missing increments.

Making doThings() static would make all the threads synchonize on a single object: TestSeven.class

Answer 2

A synchronized associated to an object's instance's method does not care of static variables, that are class's variables. Indeed, in your case, the mutated variable is a static one: x

In order to synchronize a static variable, both easiest ways in this snippet code would be to declare doThings() as static (so that it would lock on the class itself, not the object), or to lock the TestSeven.class in the case of a synchronized block:

public synchronized static void doThings() 

or

public void doThings(){
  synchronized(TestSeven.class){
    //...
  }
}

Answer 3

No, That method doThings() is not thread safe, because, it was synchronized on this instance means different object will use them as the lock to access it. So no synchronization is there. If that method doThings() uses a shared lock to access the method, then, this method is perfectly synchronized. Or, making it class level property will synchronize.