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



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



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

instance = new Object();
return instance;
}


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

387   8  

8 ответов:

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

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

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

ваш код немного сложнее, чем нужно, что может быть причиной его путаницы.

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

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

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

это должно быть

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

    return instance;
}

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

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

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

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

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

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;

}

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

private static Object instance;

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

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

начиная с 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;

}

Comments

    Ничего не найдено.