Thread-safe class in Java by means of synchronized blocks

Question

Let's say we have very simple Java class MyClass.

public class MyClass {
   private int number;

    public MyClass(int number) {
        this.number = number;
    }

    public int getNumber() {
        return number;
    }

    public void setNumber(int number) {
        this.number = number;
    }
}

There are three ways to construct thread-safe Java class which has some state:

1. Make it truly immutable
   

   public class MyClass {
      private final int number;
   
      public MyClass(int number) {
       this.number = number;
      }
   
      public int getNumber() {
       return number;
      }
   
   }
   

2. Make field number volatile.

   public class MyClass {
      private volatile int number;
   
      public MyClass(int number) {
       this.number = number;
      }
   
      public int getNumber() {
          return number;
      }
   
      public void setNumber(int number) {
          this.number = number;
      }
   }
   

3. Use a synchronized block. Classic version of this approach described in Chapter 4.3.5 of Java Concurrency in practice. And the funny thing about that it has an error in the example which is mentioned in a errata for this book.

   public class MyClass {
      private int number;
   
      public MyClass(int number) {
          setNumber(number);
      }
   
      public synchronized int getNumber() {
          return number;
      }
   
      public synchronized void setNumber(int number) {
          this.number = number;
      }
   }
   

There is one more fact that should be added to the context of discussion. In a multhithreaded environment JVM is free to reorder instructions outside of synchronized block preserving a logical sequence and happens-before relationships specified by JVM. It may cause publishing object which is not properly constructed yet to another thread.

I've got a couple of questions regarding the third case.

1. Will it be equivalent to a following piece of code:

   public class MyClass {
      private int number;
   
      public MyClass(int number) {
          synchronized (this){
              this.number = number;
          }
      }
   
      public synchronized int getNumber() {
          return number;
      }
   
      public synchronized void setNumber(int number) {
          this.number = number;
      }
   }
   

2. Will a reordering be prevented in the third case or it possible for JVM to reorder intstructions and therefore publish object with default value in field number?

3. If an answer for the second question is yes than I have one more question.

    public class MyClass {
      private int number;
   
      public MyClass(int number) {
          synchronized (new Object()){
              this.number = number;
          }
      }
   
      public synchronized int getNumber() {
          return number;
      }
   
      public synchronized void setNumber(int number) {
          this.number = number;
      }
   }
   

This strange-looking synchronized (new Object()) is supposed to prevent reordering effect. Will it work?

Just to be clear, all these examples don't have any practical applications. I'm just curious about nuances of multithreading.

Answer 1

synchronized(new Object()) will do nothing, since synchronization is only on the object you synchronize on. So if thread A synchronizes on oneObject, and thread B synchronizes on anotherObject, there is no happens-before between them. Since we can know for a fact that no other thread will ever synchronize on the new Object() you create there, this won't establish a happens-before between any other thread.

Regarding your synchronzied in the constructor, if your object is safely published to another thread, you don't need it; and if it's not, you're probably in a mess of trouble as it is. I asked this question on the concurrency-interest list a bit ago, and an interesting thread resulted. See in particular this email, which points out that even with your constructor synchronized, in the absence of safe publication another thread could see default values in your fields, and this email which (imho) ties the whole thing together.

Answer 2

In question #3, synchronized(new Object()) is a no-op and will prevent nothing. The compiler can determine that no other threads could possibly synchronize on that object (since nothing else can access the object.) This is an explicit example in Brian Goetz's paper "Java theory and practice: Synchronization optimizations in Mustang".

Even if you did need to synchronize in a constructor, and even if your synchronized(new Object()) block was useful - ie, you were synchronizing on a different long-lived object, since your other methods are synchronizing on this, you have visibility problems if you're not synchronizing on the same variable. That is to say, you do indeed want your constructor to also use synchronized(this).

An aside:

Synchronizing on this is considered poor form. Instead, synchronize on some private final field. Callers may synchronize on your object, which could lead to a deadlock. Consider the following:

public class Foo
{
    private int value;
    public synchronized int getValue() { return value; }
    public synchronized void setValue(int value) { this.value = value; }
}

public class Bar
{
    public static void deadlock()
    {
        final Foo foo = new Foo();
        synchronized(foo)
        {
            Thread t = new Thread() { public void run() { foo.setValue(1); } };
            t.start();
            t.join();
        }
    }
}

It's not obvious to callers of the Foo class that this would deadlock. Best to keep your locking semantics internal and private to your class.