What "inconsistent synchronization" means?

Question

This is my Java 1.6 class:

public class Foo {
  private ArrayList<String> names;
  public void scan() {
    if (names == null) {
      synchronized (this) {
        this.names = new ArrayList<String>();
        // fill the array with data
      }
    }
  }
}

Findbugs says:

Inconsistent synchronization of com.XXX.Foo.names; locked 40% of time

What does it mean and what I'm doing wrong? I'm trying to avoid problems when two or more clients call Foo.scan() at the same time.

Answer 1

It's beacuse you are only synchronizing when you set the names variable and not when you read it. So between the read and the write another thread could execute and you'd create two ArrayLists and fill them with data, the first one created would get GC'ed.

You need to put the synchronized block around the read and the write or add the synchronized modifier to the method.

public class Foo {
  private ArrayList<String> names;
    public void scan() {
      synchronized (this)
        if (names == null) {
           this.names = new ArrayList<String>();
           // fill the array with data
         }
       }
     }
  }

Answer 2

The first time you reference names inside scan is outside of synchronized block.
E.g., if scan is called twice from two different threads and names is null, it may go like this

1. if (names == null) from the first thread is processed (to true).
2. if (names == null) from the second thread is processed (to true).
3. First thread enters synchronized block, assignes names and leaves synchronized block.
4. Second thread enters synchronized block, assignes names and leaves synchronized block.

Now, names is initialized twice. And this is only one possible scenario where you get unexpected results.