Java: Thread safety in class with synchronized methods

I read that the following class is not thread safe since threads could read inconsistent data as there is a chance for a thread to read scaled version of real and unscaled version of imaginary. But I did not understand how.

I was under the impression that if a thread acquires a lock and is in scale() method, no other thread can be in getReal() or getImaginary() methods at the same time so that other threads cannot read 'half scaled' complex numbers. Is it not correct?

    class Complex 
    {   
         double real; 
         double imaginary;

         synchronized void scale(double scaleFactor)
         {
            real = real * scaleFactor;   
            i开发者_如何学Gomaginary = imaginary * scaleFactor; 
         }

         synchronized double getReal() 
         {
                return real;
         }

         synchronized double getImaginary() 
         {
               return imaginary;
         }
   }

---

Consider the following scenario:

1. Thread A calls getReal()
2. Thread B calls scale()
3. Thread A calls getImaginary()

This way Thread A can indeed get inconsistent real and imaginary values.

The solution would be either to

• create a common synchronized getter method to return both values at once, or
• make the class immutable, as Vivien suggested.

---

Not really a direct answer, but in your case, the best option is to make your class immutable. Each instance of Complex can't be changed after initialisation.

In this case, your scale method creates and returns a new Complex object with the new values.

Note this is how all JVM Number type works.

---

no other thread can be in getReal() or getImaginary() methods at the same time so that other threads cannot read 'half scaled' complex numbers. Is it not correct?

Yes, that's correct, however...

As Douglas points out, any client that needs access to the real and imaginary part has to perform two separate calls: one to real() and one to imaginary() (in which another thread could have called scale in between). You have no data-races, but the behavior may still depend on the scheduling.

Also, you need to make the fields private, otherwie a subclass or classes in the same package could see the "half-updated" complex number.

---

Any client of your class has to call getReal() then getImaginary() if they want to do calculations with both parts.

These calls can surround a call to scale() from another thread.

Probably the best solution is to make the Complex class immutable, as otherwise you'll end up having to make many other bits complicated to lock & unlock the objects.