Неправильная ленивая инициализация

Question

Findbug сказал мне, что я использую некорректную ленивую инициализацию.

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

    instance = new Object();
    return instance;
}

Я не вижу здесь ничего плохого.Это неправильное поведение findbug, или я что-то пропустил?

Answer 1

Findbug ссылается на потенциальную проблему с многопоточностью.В многопоточной среде ваш синглтон может быть создан несколько раз с вашим текущим кодом.

Здесь много читается , но это поможет объяснить.

Состояние гонки здесь на if check.При первом вызове поток попадет в if check, создаст экземпляр и назначит его экземпляру.Но существует вероятность, что другой поток станет активным между if check и созданием / назначением экземпляра.Этот поток также может передавать if check, потому что назначение еще не произошло.Следовательно, будет создано два (или более, если будет добавлено больше потоков) экземпляра, и ваши потоки будут иметь ссылки на разные объекты.

Answer 2

Ваш код немного сложнее, чем нужно, возможно, именно поэтому он запутался.

Редактировать: Это определенно проблема с многопоточностью, как другие опубликовали, но думал, что я опубликую здесь проверку двойной блокировки для справки ниже:

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;
}

Answer 3

ПРИМЕЧАНИЕ : решение для двойной блокировки JohnKlehm лучше. Оставив этот ответ здесь по историческим причинам.

На самом деле должно быть

public synchronized static Object getInstance() {
    if (instance == null) {
        instance = new Object();
    }

    return instance;
}

Answer 4

Вам нужно установить блокировку вокруг экземпляра, чтобы исправить это

LI: неверная ленивая инициализация статического поля (LI_LAZY_INIT_STATIC)

Этот метод содержит несинхронизированную ленивую инициализациюэнергонезависимое статическое поле.Поскольку компилятор или процессор могут переупорядочивать инструкции, потокам не гарантируется увидеть полностью инициализированный объект, если метод может быть вызван несколькими потоками.Вы можете сделать поле нестабильным, чтобы исправить проблему.Для получения дополнительной информации см. Веб-сайт модели памяти Java.

Answer 5

Вы пропустили проблему многопоточности,

private static Object instance;

public static synchronized Object getInstance() {
    return (instance != null ? instance : (instance = new Object()));
}

Answer 6

Спасибо Джону Клему за опубликованный образец

также может попытаться назначить экземпляр объекта в синхронизированном блоке напрямую

synchronized (MyCurrentClass.myLock=new Object())

, т.е.

private static volatile Object myLock = new Object();

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

    return instance;

}

Answer 7

Начиная с 1.5: экземпляр должен быть изменчивым, и вы должны интегрировать переменную tmp, чтобы избежать использования экземпляра, который создан, но его инициализация еще не завершена.

private static volatile Object myLock = new Object();
private static volatile Object instance;

public static Object getInstance() {
    if (instance == null) {
        Object tmpObj;
        synchronized (myLock) {
            tmpObj = instance;
            if (tmpObj == null) {
                tmpObj = new Object();
            }
        }
        instance = tmpObj;
    }

    return instance;

}

Answer 8

ваш статический объект не синхронизирован. Более того, ваш метод не ленивая инициализация. Обычно вы держите карту объекта и инициализируете нужный по требованию. Таким образом, вы не инициализируете их все в начале, а не вызываете их, когда это необходимо (вызывается).