Incorrect lazy initialization

Findbug told me that I use incorrect lazy initialization.

public static Object getInstance() {
    if (instance != null) {
        return instance;
    }

    instance = new Object();
    return instance;
}

I don't see anything wrong here. Is it wrong behaviour of findbug, or I missed something?

Findbug is referencing a potential threading issue. In a multi thread environment, there would be potential for your singleton to be created more than once with your current code.

There is a lot of reading here, but it will help explain.

The race condition here is on the if check. On the first call, a thread will get into the if check, and will create the instance and assign it to 'instance'. But there is potential for another thread to become active between the if check and the instance creation/assignment. This thread could also pass the if check because the assignment hasn't happened yet. Therefore, two (or more, if more threads got in) instances would be created, and your threads would have references to different objects.

Your code is slightly more complex than needed which might be why it's confused.

Edit: It's definitely the threading issue as the others posted but thought I'd post the double lock check implementation here for reference below:

private static final Object lock = new Object();
private static volatile Object instance; // must be declared volatile

public static Object getInstance() {
    if (instance == null) { // avoid sync penalty if we can
        synchronized (lock) { // declare a private static Object to use for mutex
            if (instance == null) {  // have to do this inside the sync
                instance = new Object();
            }
        }
    }

    return instance;
}